All of lore.kernel.org
 help / color / mirror / Atom feed
* userspace object manager confused
@ 2017-03-31 12:09 Dominick Grift
  2017-03-31 13:25 ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Dominick Grift @ 2017-03-31 12:09 UTC (permalink / raw)
  To: selinux

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

I vaguely recall that we discussed this issue or at least that i mentioned it here but i can't recall the outcome if any:

So today on my rawhide system i noticed that i somehow forgot to add support for the smc_socket class (i suspect that is part of the extended socket class patches)

I added the class (which i suppose is unordered like the othe extended socket classes) but as soon as I loaded up the policy with the new unordered smc_socket class the system became unusable.

This is because the dbus object manager became confused due to my adding a new (unordered) class at runtime, and that the dbus class was no longer working.

Modern systems heavily rely on dbus at the heart and so it is undesire-able that this happens.

A reboot clears this issue up but adding (unordered) classes at runtime should not cause these issues i suspect

-- 
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] 14+ messages in thread

* Re: userspace object manager confused
  2017-03-31 12:09 userspace object manager confused Dominick Grift
@ 2017-03-31 13:25 ` Stephen Smalley
  2017-03-31 13:30   ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2017-03-31 13:25 UTC (permalink / raw)
  To: Dominick Grift, selinux

On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> I vaguely recall that we discussed this issue or at least that i
> mentioned it here but i can't recall the outcome if any:
> 
> So today on my rawhide system i noticed that i somehow forgot to add
> support for the smc_socket class (i suspect that is part of the
> extended socket class patches)
> 
> I added the class (which i suppose is unordered like the othe
> extended socket classes) but as soon as I loaded up the policy with
> the new unordered smc_socket class the system became unusable.
> 
> This is because the dbus object manager became confused due to my
> adding a new (unordered) class at runtime, and that the dbus class
> was no longer working.
> 
> Modern systems heavily rely on dbus at the heart and so it is
> undesire-able that this happens.
> 
> A reboot clears this issue up but adding (unordered) classes at
> runtime should not cause these issues i suspect

dbusd doesn't use selinux_check_access() and therefore does not yet
support reordering of their classes/permissions at runtime.  The same
is true of all userspace object managers created before
selinux_check_access() was introduced - anything that directly calls
security_compute_av() or avc_has_perm(). dbusd does call
selinux_set_mapping() at startup, so it can correctly handle
reordering of classes/permissions across restarts, but not while it is
running.  Calling selinux_set_mapping() again upon policy reloads (e.g.
from policy_reload_callback() if (event == AVC_CALLBACK_RESET) before
returning) may fix this problem, but requires proper locking.  Even
better would be to rid the dbusd selinux implementation of threading
entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92831#c4

smc_socket was added by the kernel developers as part of the merge with
net-next since we now trigger a build failure in the kernel if any new
address families are introduced without adding a corresponding security
class (so that SELinux always supports a separate class per network
address family going forward). So there have been no policy patches
submitted yet to define it in refpolicy even AFAIK.

[1] https://github.com/SELinuxProject/selinux/issues/34

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

* Re: userspace object manager confused
  2017-03-31 13:25 ` Stephen Smalley
@ 2017-03-31 13:30   ` Stephen Smalley
  2017-03-31 13:36     ` Dominick Grift
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2017-03-31 13:30 UTC (permalink / raw)
  To: Dominick Grift, selinux

On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > I vaguely recall that we discussed this issue or at least that i
> > mentioned it here but i can't recall the outcome if any:
> > 
> > So today on my rawhide system i noticed that i somehow forgot to
> > add
> > support for the smc_socket class (i suspect that is part of the
> > extended socket class patches)
> > 
> > I added the class (which i suppose is unordered like the othe
> > extended socket classes) but as soon as I loaded up the policy with
> > the new unordered smc_socket class the system became unusable.
> > 
> > This is because the dbus object manager became confused due to my
> > adding a new (unordered) class at runtime, and that the dbus class
> > was no longer working.
> > 
> > Modern systems heavily rely on dbus at the heart and so it is
> > undesire-able that this happens.
> > 
> > A reboot clears this issue up but adding (unordered) classes at
> > runtime should not cause these issues i suspect
> 
> dbusd doesn't use selinux_check_access() and therefore does not yet
> support reordering of their classes/permissions at runtime.  The same
> is true of all userspace object managers created before
> selinux_check_access() was introduced - anything that directly calls
> security_compute_av() or avc_has_perm(). dbusd does call
> selinux_set_mapping() at startup, so it can correctly handle
> reordering of classes/permissions across restarts, but not while it
> is
> running.  Calling selinux_set_mapping() again upon policy reloads
> (e.g.
> from policy_reload_callback() if (event == AVC_CALLBACK_RESET) before
> returning) may fix this problem, but requires proper locking.  Even
> better would be to rid the dbusd selinux implementation of threading
> entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92831#c4
> 
> smc_socket was added by the kernel developers as part of the merge
> with
> net-next since we now trigger a build failure in the kernel if any
> new
> address families are introduced without adding a corresponding
> security
> class (so that SELinux always supports a separate class per network
> address family going forward). So there have been no policy patches
> submitted yet to define it in refpolicy even AFAIK.
> 
> [1] https://github.com/SELinuxProject/selinux/issues/34

BTW, I'm not sure what you did to trigger the problem.  When I tested
the extended socket classes, I added them to my running policy via a
CIL module like this:

(policycap extended_socket_class)

(classcommon sctp_socket socket)
(class sctp_socket (node_bind))

<snip>

(classcommon qipcrtr_socket socket)
(class qipcrtr_socket ())

(classorder (unordered sctp_socket icmp_socket ax25_socket ipx_socket
netrom_socket bridge_socket atmpvc_socket x25_socket rose_socket
decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
llc_socket ib_socket mpls_socket can_socket tipc_socket
bluetooth_socket iucv_socket rxrpc_socket isdn_socket phonet_socket
ieee802154_socket caif_socket alg_socket nfc_socket vsock_socket
kcm_socket qipcrtr_socket))

The classorder statement at the end ensured that they were appended to
the end of the class list and therefore did not break anything.

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

* Re: userspace object manager confused
  2017-03-31 13:30   ` Stephen Smalley
@ 2017-03-31 13:36     ` Dominick Grift
  2017-03-31 13:53       ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Dominick Grift @ 2017-03-31 13:36 UTC (permalink / raw)
  To: selinux

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

On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
> On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> > On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > > I vaguely recall that we discussed this issue or at least that i
> > > mentioned it here but i can't recall the outcome if any:
> > > 
> > > So today on my rawhide system i noticed that i somehow forgot to
> > > add
> > > support for the smc_socket class (i suspect that is part of the
> > > extended socket class patches)
> > > 
> > > I added the class (which i suppose is unordered like the othe
> > > extended socket classes) but as soon as I loaded up the policy with
> > > the new unordered smc_socket class the system became unusable.
> > > 
> > > This is because the dbus object manager became confused due to my
> > > adding a new (unordered) class at runtime, and that the dbus class
> > > was no longer working.
> > > 
> > > Modern systems heavily rely on dbus at the heart and so it is
> > > undesire-able that this happens.
> > > 
> > > A reboot clears this issue up but adding (unordered) classes at
> > > runtime should not cause these issues i suspect
> > 
> > dbusd doesn't use selinux_check_access() and therefore does not yet
> > support reordering of their classes/permissions at runtime.  The same
> > is true of all userspace object managers created before
> > selinux_check_access() was introduced - anything that directly calls
> > security_compute_av() or avc_has_perm(). dbusd does call
> > selinux_set_mapping() at startup, so it can correctly handle
> > reordering of classes/permissions across restarts, but not while it
> > is
> > running.  Calling selinux_set_mapping() again upon policy reloads
> > (e.g.
> > from policy_reload_callback() if (event == AVC_CALLBACK_RESET) before
> > returning) may fix this problem, but requires proper locking.  Even
> > better would be to rid the dbusd selinux implementation of threading
> > entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92831#c4

Thank you, I suppose i should take this to them then.

> > 
> > smc_socket was added by the kernel developers as part of the merge
> > with
> > net-next since we now trigger a build failure in the kernel if any
> > new
> > address families are introduced without adding a corresponding
> > security
> > class (so that SELinux always supports a separate class per network
> > address family going forward). So there have been no policy patches
> > submitted yet to define it in refpolicy even AFAIK.
> > 
> > [1] https://github.com/SELinuxProject/selinux/issues/34

So i suppose its an unordered class? or do i have to order this one to avoid future issues?

> 
> BTW, I'm not sure what you did to trigger the problem.  When I tested
> the extended socket classes, I added them to my running policy via a
> CIL module like this:

Well i just added this commit (ignore the typo in the name) which adds the new class to the unordered socket list

https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659f347863f09ec9caeceb56

then i built rpms using this script:

https://github.com/DefenSec/dssp2-standard/blob/master/support/rpm/dssp2-standard.sh

and just rpm -Uvh the new rpm

> 
> (policycap extended_socket_class)
> 
> (classcommon sctp_socket socket)
> (class sctp_socket (node_bind))
> 
> <snip>
> 
> (classcommon qipcrtr_socket socket)
> (class qipcrtr_socket ())
> 
> (classorder (unordered sctp_socket icmp_socket ax25_socket ipx_socket
> netrom_socket bridge_socket atmpvc_socket x25_socket rose_socket
> decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
> llc_socket ib_socket mpls_socket can_socket tipc_socket
> bluetooth_socket iucv_socket rxrpc_socket isdn_socket phonet_socket
> ieee802154_socket caif_socket alg_socket nfc_socket vsock_socket
> kcm_socket qipcrtr_socket))
> 
> The classorder statement at the end ensured that they were appended to
> the end of the class list and therefore did not break anything.
> 

-- 
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] 14+ messages in thread

* Re: userspace object manager confused
  2017-03-31 13:36     ` Dominick Grift
@ 2017-03-31 13:53       ` Stephen Smalley
  2017-03-31 13:59         ` Dominick Grift
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2017-03-31 13:53 UTC (permalink / raw)
  To: Dominick Grift, selinux

On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
> On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
> > On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> > > On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > > > I vaguely recall that we discussed this issue or at least that
> > > > i
> > > > mentioned it here but i can't recall the outcome if any:
> > > > 
> > > > So today on my rawhide system i noticed that i somehow forgot
> > > > to
> > > > add
> > > > support for the smc_socket class (i suspect that is part of the
> > > > extended socket class patches)
> > > > 
> > > > I added the class (which i suppose is unordered like the othe
> > > > extended socket classes) but as soon as I loaded up the policy
> > > > with
> > > > the new unordered smc_socket class the system became unusable.
> > > > 
> > > > This is because the dbus object manager became confused due to
> > > > my
> > > > adding a new (unordered) class at runtime, and that the dbus
> > > > class
> > > > was no longer working.
> > > > 
> > > > Modern systems heavily rely on dbus at the heart and so it is
> > > > undesire-able that this happens.
> > > > 
> > > > A reboot clears this issue up but adding (unordered) classes at
> > > > runtime should not cause these issues i suspect
> > > 
> > > dbusd doesn't use selinux_check_access() and therefore does not
> > > yet
> > > support reordering of their classes/permissions at runtime.  The
> > > same
> > > is true of all userspace object managers created before
> > > selinux_check_access() was introduced - anything that directly
> > > calls
> > > security_compute_av() or avc_has_perm(). dbusd does call
> > > selinux_set_mapping() at startup, so it can correctly handle
> > > reordering of classes/permissions across restarts, but not while
> > > it
> > > is
> > > running.  Calling selinux_set_mapping() again upon policy reloads
> > > (e.g.
> > > from policy_reload_callback() if (event == AVC_CALLBACK_RESET)
> > > before
> > > returning) may fix this problem, but requires proper
> > > locking.  Even
> > > better would be to rid the dbusd selinux implementation of
> > > threading
> > > entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92831#
> > > c4
> 
> Thank you, I suppose i should take this to them then.

No, someone who uses SELinux will need to develop a patch and test it
for them; dbusd maintainer doesn't use SELinux himself.

Looks like I'm also wrong about being able to call
selinux_set_mapping() from an AVC callback, since selinux_set_mapping()
itself calls avc_reset() to flush any cache entries before reloading
the mapping, so that would produce infinite recursion.  Sigh.  Could
change selinux_set_mapping() to not do that in libselinux, but that
won't help on existing releases.  Maybe we should just convert it over
to using selinux_check_access() and drop all direct usage of the AVC.

> > > 
> > > smc_socket was added by the kernel developers as part of the
> > > merge
> > > with
> > > net-next since we now trigger a build failure in the kernel if
> > > any
> > > new
> > > address families are introduced without adding a corresponding
> > > security
> > > class (so that SELinux always supports a separate class per
> > > network
> > > address family going forward). So there have been no policy
> > > patches
> > > submitted yet to define it in refpolicy even AFAIK.
> > > 
> > > [1] https://github.com/SELinuxProject/selinux/issues/34
> 
> So i suppose its an unordered class? or do i have to order this one
> to avoid future issues?
> 
> > 
> > BTW, I'm not sure what you did to trigger the problem.  When I
> > tested
> > the extended socket classes, I added them to my running policy via
> > a
> > CIL module like this:
> 
> Well i just added this commit (ignore the typo in the name) which
> adds the new class to the unordered socket list
> 
> https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659f347
> 863f09ec9caeceb56
> 
> then i built rpms using this script:
> 
> https://github.com/DefenSec/dssp2-standard/blob/master/support/rpm/ds
> sp2-standard.sh
> 
> and just rpm -Uvh the new rpm
> 
> > 
> > (policycap extended_socket_class)
> > 
> > (classcommon sctp_socket socket)
> > (class sctp_socket (node_bind))
> > 
> > <snip>
> > 
> > (classcommon qipcrtr_socket socket)
> > (class qipcrtr_socket ())
> > 
> > (classorder (unordered sctp_socket icmp_socket ax25_socket
> > ipx_socket
> > netrom_socket bridge_socket atmpvc_socket x25_socket rose_socket
> > decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
> > llc_socket ib_socket mpls_socket can_socket tipc_socket
> > bluetooth_socket iucv_socket rxrpc_socket isdn_socket phonet_socket
> > ieee802154_socket caif_socket alg_socket nfc_socket vsock_socket
> > kcm_socket qipcrtr_socket))
> > 
> > The classorder statement at the end ensured that they were appended
> > to
> > the end of the class list and therefore did not break anything.
> > 

Looks like you failed to include a classorder statement for it as I did
above.  That's still required to avoid breaking legacy userspace object
managers.

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

* Re: userspace object manager confused
  2017-03-31 13:53       ` Stephen Smalley
@ 2017-03-31 13:59         ` Dominick Grift
  2017-03-31 14:10           ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Dominick Grift @ 2017-03-31 13:59 UTC (permalink / raw)
  To: selinux

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

On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
> On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
> > On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
> > > On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> > > > On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > > > > I vaguely recall that we discussed this issue or at least that
> > > > > i
> > > > > mentioned it here but i can't recall the outcome if any:
> > > > > 
> > > > > So today on my rawhide system i noticed that i somehow forgot
> > > > > to
> > > > > add
> > > > > support for the smc_socket class (i suspect that is part of the
> > > > > extended socket class patches)
> > > > > 
> > > > > I added the class (which i suppose is unordered like the othe
> > > > > extended socket classes) but as soon as I loaded up the policy
> > > > > with
> > > > > the new unordered smc_socket class the system became unusable.
> > > > > 
> > > > > This is because the dbus object manager became confused due to
> > > > > my
> > > > > adding a new (unordered) class at runtime, and that the dbus
> > > > > class
> > > > > was no longer working.
> > > > > 
> > > > > Modern systems heavily rely on dbus at the heart and so it is
> > > > > undesire-able that this happens.
> > > > > 
> > > > > A reboot clears this issue up but adding (unordered) classes at
> > > > > runtime should not cause these issues i suspect
> > > > 
> > > > dbusd doesn't use selinux_check_access() and therefore does not
> > > > yet
> > > > support reordering of their classes/permissions at runtime.  The
> > > > same
> > > > is true of all userspace object managers created before
> > > > selinux_check_access() was introduced - anything that directly
> > > > calls
> > > > security_compute_av() or avc_has_perm(). dbusd does call
> > > > selinux_set_mapping() at startup, so it can correctly handle
> > > > reordering of classes/permissions across restarts, but not while
> > > > it
> > > > is
> > > > running.  Calling selinux_set_mapping() again upon policy reloads
> > > > (e.g.
> > > > from policy_reload_callback() if (event == AVC_CALLBACK_RESET)
> > > > before
> > > > returning) may fix this problem, but requires proper
> > > > locking.  Even
> > > > better would be to rid the dbusd selinux implementation of
> > > > threading
> > > > entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92831#
> > > > c4
> > 
> > Thank you, I suppose i should take this to them then.
> 
> No, someone who uses SELinux will need to develop a patch and test it
> for them; dbusd maintainer doesn't use SELinux himself.
> 
> Looks like I'm also wrong about being able to call
> selinux_set_mapping() from an AVC callback, since selinux_set_mapping()
> itself calls avc_reset() to flush any cache entries before reloading
> the mapping, so that would produce infinite recursion.  Sigh.  Could
> change selinux_set_mapping() to not do that in libselinux, but that
> won't help on existing releases.  Maybe we should just convert it over
> to using selinux_check_access() and drop all direct usage of the AVC.
> 
> > > > 
> > > > smc_socket was added by the kernel developers as part of the
> > > > merge
> > > > with
> > > > net-next since we now trigger a build failure in the kernel if
> > > > any
> > > > new
> > > > address families are introduced without adding a corresponding
> > > > security
> > > > class (so that SELinux always supports a separate class per
> > > > network
> > > > address family going forward). So there have been no policy
> > > > patches
> > > > submitted yet to define it in refpolicy even AFAIK.
> > > > 
> > > > [1] https://github.com/SELinuxProject/selinux/issues/34
> > 
> > So i suppose its an unordered class? or do i have to order this one
> > to avoid future issues?
> > 
> > > 
> > > BTW, I'm not sure what you did to trigger the problem.  When I
> > > tested
> > > the extended socket classes, I added them to my running policy via
> > > a
> > > CIL module like this:
> > 
> > Well i just added this commit (ignore the typo in the name) which
> > adds the new class to the unordered socket list
> > 
> > https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659f347
> > 863f09ec9caeceb56
> > 
> > then i built rpms using this script:
> > 
> > https://github.com/DefenSec/dssp2-standard/blob/master/support/rpm/ds
> > sp2-standard.sh
> > 
> > and just rpm -Uvh the new rpm
> > 
> > > 
> > > (policycap extended_socket_class)
> > > 
> > > (classcommon sctp_socket socket)
> > > (class sctp_socket (node_bind))
> > > 
> > > <snip>
> > > 
> > > (classcommon qipcrtr_socket socket)
> > > (class qipcrtr_socket ())
> > > 
> > > (classorder (unordered sctp_socket icmp_socket ax25_socket
> > > ipx_socket
> > > netrom_socket bridge_socket atmpvc_socket x25_socket rose_socket
> > > decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
> > > llc_socket ib_socket mpls_socket can_socket tipc_socket
> > > bluetooth_socket iucv_socket rxrpc_socket isdn_socket phonet_socket
> > > ieee802154_socket caif_socket alg_socket nfc_socket vsock_socket
> > > kcm_socket qipcrtr_socket))
> > > 
> > > The classorder statement at the end ensured that they were appended
> > > to
> > > the end of the class list and therefore did not break anything.
> > > 
> 
> Looks like you failed to include a classorder statement for it as I did
> above.  That's still required to avoid breaking legacy userspace object
> managers.
> 
> 

No I added "scm_socket" (i renamed it later to smc_socket) to my list or unordered classorder:

;
; Class order of unordered Linux access vectors
;

(classorder
    (unordered
        alg_socket
        atmpvc_socket
        atmsvc_socket
        ax25_socket
        bluetooth_socket
        caif_socket
        can_socket
        decnet_socket
        icmp_socket
        ieee802154_socket
        ipx_socket
        irda_socket
        isdn_socket
        iucv_socket
        kcm_socket
        llc_socket
        netrom_socket
        nfc_socket
        phonet_socket
        pppox_socket
        qipcrtr_socket
        rds_socket
        rose_socket
        rxrpc_socket
        scm_socket
        sctp_socket
        tipc_socket
        vsock_socket
        x25_socket
    )
)


-- 
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] 14+ messages in thread

* Re: userspace object manager confused
  2017-03-31 13:59         ` Dominick Grift
@ 2017-03-31 14:10           ` Stephen Smalley
  2017-03-31 14:12             ` James Carter
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2017-03-31 14:10 UTC (permalink / raw)
  To: Dominick Grift, selinux

On Fri, 2017-03-31 at 15:59 +0200, Dominick Grift wrote:
> On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
> > On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
> > > On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
> > > > On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> > > > > On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > > > > > I vaguely recall that we discussed this issue or at least
> > > > > > that
> > > > > > i
> > > > > > mentioned it here but i can't recall the outcome if any:
> > > > > > 
> > > > > > So today on my rawhide system i noticed that i somehow
> > > > > > forgot
> > > > > > to
> > > > > > add
> > > > > > support for the smc_socket class (i suspect that is part of
> > > > > > the
> > > > > > extended socket class patches)
> > > > > > 
> > > > > > I added the class (which i suppose is unordered like the
> > > > > > othe
> > > > > > extended socket classes) but as soon as I loaded up the
> > > > > > policy
> > > > > > with
> > > > > > the new unordered smc_socket class the system became
> > > > > > unusable.
> > > > > > 
> > > > > > This is because the dbus object manager became confused due
> > > > > > to
> > > > > > my
> > > > > > adding a new (unordered) class at runtime, and that the
> > > > > > dbus
> > > > > > class
> > > > > > was no longer working.
> > > > > > 
> > > > > > Modern systems heavily rely on dbus at the heart and so it
> > > > > > is
> > > > > > undesire-able that this happens.
> > > > > > 
> > > > > > A reboot clears this issue up but adding (unordered)
> > > > > > classes at
> > > > > > runtime should not cause these issues i suspect
> > > > > 
> > > > > dbusd doesn't use selinux_check_access() and therefore does
> > > > > not
> > > > > yet
> > > > > support reordering of their classes/permissions at
> > > > > runtime.  The
> > > > > same
> > > > > is true of all userspace object managers created before
> > > > > selinux_check_access() was introduced - anything that
> > > > > directly
> > > > > calls
> > > > > security_compute_av() or avc_has_perm(). dbusd does call
> > > > > selinux_set_mapping() at startup, so it can correctly handle
> > > > > reordering of classes/permissions across restarts, but not
> > > > > while
> > > > > it
> > > > > is
> > > > > running.  Calling selinux_set_mapping() again upon policy
> > > > > reloads
> > > > > (e.g.
> > > > > from policy_reload_callback() if (event ==
> > > > > AVC_CALLBACK_RESET)
> > > > > before
> > > > > returning) may fix this problem, but requires proper
> > > > > locking.  Even
> > > > > better would be to rid the dbusd selinux implementation of
> > > > > threading
> > > > > entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92
> > > > > 831#
> > > > > c4
> > > 
> > > Thank you, I suppose i should take this to them then.
> > 
> > No, someone who uses SELinux will need to develop a patch and test
> > it
> > for them; dbusd maintainer doesn't use SELinux himself.
> > 
> > Looks like I'm also wrong about being able to call
> > selinux_set_mapping() from an AVC callback, since
> > selinux_set_mapping()
> > itself calls avc_reset() to flush any cache entries before
> > reloading
> > the mapping, so that would produce infinite
> > recursion.  Sigh.  Could
> > change selinux_set_mapping() to not do that in libselinux, but that
> > won't help on existing releases.  Maybe we should just convert it
> > over
> > to using selinux_check_access() and drop all direct usage of the
> > AVC.
> > 
> > > > > 
> > > > > smc_socket was added by the kernel developers as part of the
> > > > > merge
> > > > > with
> > > > > net-next since we now trigger a build failure in the kernel
> > > > > if
> > > > > any
> > > > > new
> > > > > address families are introduced without adding a
> > > > > corresponding
> > > > > security
> > > > > class (so that SELinux always supports a separate class per
> > > > > network
> > > > > address family going forward). So there have been no policy
> > > > > patches
> > > > > submitted yet to define it in refpolicy even AFAIK.
> > > > > 
> > > > > [1] https://github.com/SELinuxProject/selinux/issues/34
> > > 
> > > So i suppose its an unordered class? or do i have to order this
> > > one
> > > to avoid future issues?
> > > 
> > > > 
> > > > BTW, I'm not sure what you did to trigger the problem.  When I
> > > > tested
> > > > the extended socket classes, I added them to my running policy
> > > > via
> > > > a
> > > > CIL module like this:
> > > 
> > > Well i just added this commit (ignore the typo in the name) which
> > > adds the new class to the unordered socket list
> > > 
> > > https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659
> > > f347
> > > 863f09ec9caeceb56
> > > 
> > > then i built rpms using this script:
> > > 
> > > https://github.com/DefenSec/dssp2-standard/blob/master/support/rp
> > > m/ds
> > > sp2-standard.sh
> > > 
> > > and just rpm -Uvh the new rpm
> > > 
> > > > 
> > > > (policycap extended_socket_class)
> > > > 
> > > > (classcommon sctp_socket socket)
> > > > (class sctp_socket (node_bind))
> > > > 
> > > > <snip>
> > > > 
> > > > (classcommon qipcrtr_socket socket)
> > > > (class qipcrtr_socket ())
> > > > 
> > > > (classorder (unordered sctp_socket icmp_socket ax25_socket
> > > > ipx_socket
> > > > netrom_socket bridge_socket atmpvc_socket x25_socket
> > > > rose_socket
> > > > decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
> > > > llc_socket ib_socket mpls_socket can_socket tipc_socket
> > > > bluetooth_socket iucv_socket rxrpc_socket isdn_socket
> > > > phonet_socket
> > > > ieee802154_socket caif_socket alg_socket nfc_socket
> > > > vsock_socket
> > > > kcm_socket qipcrtr_socket))
> > > > 
> > > > The classorder statement at the end ensured that they were
> > > > appended
> > > > to
> > > > the end of the class list and therefore did not break anything.
> > > > 
> > 
> > Looks like you failed to include a classorder statement for it as I
> > did
> > above.  That's still required to avoid breaking legacy userspace
> > object
> > managers.
> > 
> > 
> 
> No I added "scm_socket" (i renamed it later to smc_socket) to my list
> or unordered classorder:
> 
> ;
> ; Class order of unordered Linux access vectors
> ;
> 
> (classorder
>     (unordered
>         alg_socket
>         atmpvc_socket
>         atmsvc_socket
>         ax25_socket
>         bluetooth_socket
>         caif_socket
>         can_socket
>         decnet_socket
>         icmp_socket
>         ieee802154_socket
>         ipx_socket
>         irda_socket
>         isdn_socket
>         iucv_socket
>         kcm_socket
>         llc_socket
>         netrom_socket
>         nfc_socket
>         phonet_socket
>         pppox_socket
>         qipcrtr_socket
>         rds_socket
>         rose_socket
>         rxrpc_socket
>         scm_socket
>         sctp_socket
>         tipc_socket
>         vsock_socket
>         x25_socket
>     )
> )

Then I'm confused as to why you encountered breakage.  Doesn't CIL
ensure that all of those classes are appended to the existing class
list?  In which case it won't disturb the dbus class definitions.

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

* Re: userspace object manager confused
  2017-03-31 14:10           ` Stephen Smalley
@ 2017-03-31 14:12             ` James Carter
  2017-03-31 14:17               ` Dominick Grift
  2017-03-31 14:32               ` Dominick Grift
  0 siblings, 2 replies; 14+ messages in thread
From: James Carter @ 2017-03-31 14:12 UTC (permalink / raw)
  To: Stephen Smalley, Dominick Grift, selinux

On 03/31/2017 10:10 AM, Stephen Smalley wrote:
> On Fri, 2017-03-31 at 15:59 +0200, Dominick Grift wrote:
>> On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
>>> On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
>>>> On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
>>>>> On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
>>>>>> On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
>>>>>>> I vaguely recall that we discussed this issue or at least
>>>>>>> that
>>>>>>> i
>>>>>>> mentioned it here but i can't recall the outcome if any:
>>>>>>>
>>>>>>> So today on my rawhide system i noticed that i somehow
>>>>>>> forgot
>>>>>>> to
>>>>>>> add
>>>>>>> support for the smc_socket class (i suspect that is part of
>>>>>>> the
>>>>>>> extended socket class patches)
>>>>>>>
>>>>>>> I added the class (which i suppose is unordered like the
>>>>>>> othe
>>>>>>> extended socket classes) but as soon as I loaded up the
>>>>>>> policy
>>>>>>> with
>>>>>>> the new unordered smc_socket class the system became
>>>>>>> unusable.
>>>>>>>
>>>>>>> This is because the dbus object manager became confused due
>>>>>>> to
>>>>>>> my
>>>>>>> adding a new (unordered) class at runtime, and that the
>>>>>>> dbus
>>>>>>> class
>>>>>>> was no longer working.
>>>>>>>
>>>>>>> Modern systems heavily rely on dbus at the heart and so it
>>>>>>> is
>>>>>>> undesire-able that this happens.
>>>>>>>
>>>>>>> A reboot clears this issue up but adding (unordered)
>>>>>>> classes at
>>>>>>> runtime should not cause these issues i suspect
>>>>>>
>>>>>> dbusd doesn't use selinux_check_access() and therefore does
>>>>>> not
>>>>>> yet
>>>>>> support reordering of their classes/permissions at
>>>>>> runtime.  The
>>>>>> same
>>>>>> is true of all userspace object managers created before
>>>>>> selinux_check_access() was introduced - anything that
>>>>>> directly
>>>>>> calls
>>>>>> security_compute_av() or avc_has_perm(). dbusd does call
>>>>>> selinux_set_mapping() at startup, so it can correctly handle
>>>>>> reordering of classes/permissions across restarts, but not
>>>>>> while
>>>>>> it
>>>>>> is
>>>>>> running.  Calling selinux_set_mapping() again upon policy
>>>>>> reloads
>>>>>> (e.g.
>>>>>> from policy_reload_callback() if (event ==
>>>>>> AVC_CALLBACK_RESET)
>>>>>> before
>>>>>> returning) may fix this problem, but requires proper
>>>>>> locking.  Even
>>>>>> better would be to rid the dbusd selinux implementation of
>>>>>> threading
>>>>>> entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92
>>>>>> 831#
>>>>>> c4
>>>>
>>>> Thank you, I suppose i should take this to them then.
>>>
>>> No, someone who uses SELinux will need to develop a patch and test
>>> it
>>> for them; dbusd maintainer doesn't use SELinux himself.
>>>
>>> Looks like I'm also wrong about being able to call
>>> selinux_set_mapping() from an AVC callback, since
>>> selinux_set_mapping()
>>> itself calls avc_reset() to flush any cache entries before
>>> reloading
>>> the mapping, so that would produce infinite
>>> recursion.  Sigh.  Could
>>> change selinux_set_mapping() to not do that in libselinux, but that
>>> won't help on existing releases.  Maybe we should just convert it
>>> over
>>> to using selinux_check_access() and drop all direct usage of the
>>> AVC.
>>>
>>>>>>
>>>>>> smc_socket was added by the kernel developers as part of the
>>>>>> merge
>>>>>> with
>>>>>> net-next since we now trigger a build failure in the kernel
>>>>>> if
>>>>>> any
>>>>>> new
>>>>>> address families are introduced without adding a
>>>>>> corresponding
>>>>>> security
>>>>>> class (so that SELinux always supports a separate class per
>>>>>> network
>>>>>> address family going forward). So there have been no policy
>>>>>> patches
>>>>>> submitted yet to define it in refpolicy even AFAIK.
>>>>>>
>>>>>> [1] https://github.com/SELinuxProject/selinux/issues/34
>>>>
>>>> So i suppose its an unordered class? or do i have to order this
>>>> one
>>>> to avoid future issues?
>>>>
>>>>>
>>>>> BTW, I'm not sure what you did to trigger the problem.  When I
>>>>> tested
>>>>> the extended socket classes, I added them to my running policy
>>>>> via
>>>>> a
>>>>> CIL module like this:
>>>>
>>>> Well i just added this commit (ignore the typo in the name) which
>>>> adds the new class to the unordered socket list
>>>>
>>>> https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659
>>>> f347
>>>> 863f09ec9caeceb56
>>>>
>>>> then i built rpms using this script:
>>>>
>>>> https://github.com/DefenSec/dssp2-standard/blob/master/support/rp
>>>> m/ds
>>>> sp2-standard.sh
>>>>
>>>> and just rpm -Uvh the new rpm
>>>>
>>>>>
>>>>> (policycap extended_socket_class)
>>>>>
>>>>> (classcommon sctp_socket socket)
>>>>> (class sctp_socket (node_bind))
>>>>>
>>>>> <snip>
>>>>>
>>>>> (classcommon qipcrtr_socket socket)
>>>>> (class qipcrtr_socket ())
>>>>>
>>>>> (classorder (unordered sctp_socket icmp_socket ax25_socket
>>>>> ipx_socket
>>>>> netrom_socket bridge_socket atmpvc_socket x25_socket
>>>>> rose_socket
>>>>> decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
>>>>> llc_socket ib_socket mpls_socket can_socket tipc_socket
>>>>> bluetooth_socket iucv_socket rxrpc_socket isdn_socket
>>>>> phonet_socket
>>>>> ieee802154_socket caif_socket alg_socket nfc_socket
>>>>> vsock_socket
>>>>> kcm_socket qipcrtr_socket))
>>>>>
>>>>> The classorder statement at the end ensured that they were
>>>>> appended
>>>>> to
>>>>> the end of the class list and therefore did not break anything.
>>>>>
>>>
>>> Looks like you failed to include a classorder statement for it as I
>>> did
>>> above.  That's still required to avoid breaking legacy userspace
>>> object
>>> managers.
>>>
>>>
>>
>> No I added "scm_socket" (i renamed it later to smc_socket) to my list
>> or unordered classorder:
>>
>> ;
>> ; Class order of unordered Linux access vectors
>> ;
>>
>> (classorder
>>     (unordered
>>         alg_socket
>>         atmpvc_socket
>>         atmsvc_socket
>>         ax25_socket
>>         bluetooth_socket
>>         caif_socket
>>         can_socket
>>         decnet_socket
>>         icmp_socket
>>         ieee802154_socket
>>         ipx_socket
>>         irda_socket
>>         isdn_socket
>>         iucv_socket
>>         kcm_socket
>>         llc_socket
>>         netrom_socket
>>         nfc_socket
>>         phonet_socket
>>         pppox_socket
>>         qipcrtr_socket
>>         rds_socket
>>         rose_socket
>>         rxrpc_socket
>>         scm_socket
>>         sctp_socket
>>         tipc_socket
>>         vsock_socket
>>         x25_socket
>>     )
>> )
>
> Then I'm confused as to why you encountered breakage.  Doesn't CIL
> ensure that all of those classes are appended to the existing class
> list?  In which case it won't disturb the dbus class definitions.
>

CIL appends unordered classes to the existing class list, so I am not sure what 
is going on.

Jim



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

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

* Re: userspace object manager confused
  2017-03-31 14:12             ` James Carter
@ 2017-03-31 14:17               ` Dominick Grift
  2017-03-31 14:30                 ` James Carter
  2017-03-31 14:32               ` Dominick Grift
  1 sibling, 1 reply; 14+ messages in thread
From: Dominick Grift @ 2017-03-31 14:17 UTC (permalink / raw)
  To: selinux

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

On Fri, Mar 31, 2017 at 10:12:50AM -0400, James Carter wrote:
> On 03/31/2017 10:10 AM, Stephen Smalley wrote:
> > On Fri, 2017-03-31 at 15:59 +0200, Dominick Grift wrote:
> > > On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
> > > > On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
> > > > > On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
> > > > > > On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> > > > > > > On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > > > > > > > I vaguely recall that we discussed this issue or at least
> > > > > > > > that
> > > > > > > > i
> > > > > > > > mentioned it here but i can't recall the outcome if any:
> > > > > > > > 
> > > > > > > > So today on my rawhide system i noticed that i somehow
> > > > > > > > forgot
> > > > > > > > to
> > > > > > > > add
> > > > > > > > support for the smc_socket class (i suspect that is part of
> > > > > > > > the
> > > > > > > > extended socket class patches)
> > > > > > > > 
> > > > > > > > I added the class (which i suppose is unordered like the
> > > > > > > > othe
> > > > > > > > extended socket classes) but as soon as I loaded up the
> > > > > > > > policy
> > > > > > > > with
> > > > > > > > the new unordered smc_socket class the system became
> > > > > > > > unusable.
> > > > > > > > 
> > > > > > > > This is because the dbus object manager became confused due
> > > > > > > > to
> > > > > > > > my
> > > > > > > > adding a new (unordered) class at runtime, and that the
> > > > > > > > dbus
> > > > > > > > class
> > > > > > > > was no longer working.
> > > > > > > > 
> > > > > > > > Modern systems heavily rely on dbus at the heart and so it
> > > > > > > > is
> > > > > > > > undesire-able that this happens.
> > > > > > > > 
> > > > > > > > A reboot clears this issue up but adding (unordered)
> > > > > > > > classes at
> > > > > > > > runtime should not cause these issues i suspect
> > > > > > > 
> > > > > > > dbusd doesn't use selinux_check_access() and therefore does
> > > > > > > not
> > > > > > > yet
> > > > > > > support reordering of their classes/permissions at
> > > > > > > runtime.  The
> > > > > > > same
> > > > > > > is true of all userspace object managers created before
> > > > > > > selinux_check_access() was introduced - anything that
> > > > > > > directly
> > > > > > > calls
> > > > > > > security_compute_av() or avc_has_perm(). dbusd does call
> > > > > > > selinux_set_mapping() at startup, so it can correctly handle
> > > > > > > reordering of classes/permissions across restarts, but not
> > > > > > > while
> > > > > > > it
> > > > > > > is
> > > > > > > running.  Calling selinux_set_mapping() again upon policy
> > > > > > > reloads
> > > > > > > (e.g.
> > > > > > > from policy_reload_callback() if (event ==
> > > > > > > AVC_CALLBACK_RESET)
> > > > > > > before
> > > > > > > returning) may fix this problem, but requires proper
> > > > > > > locking.  Even
> > > > > > > better would be to rid the dbusd selinux implementation of
> > > > > > > threading
> > > > > > > entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92
> > > > > > > 831#
> > > > > > > c4
> > > > > 
> > > > > Thank you, I suppose i should take this to them then.
> > > > 
> > > > No, someone who uses SELinux will need to develop a patch and test
> > > > it
> > > > for them; dbusd maintainer doesn't use SELinux himself.
> > > > 
> > > > Looks like I'm also wrong about being able to call
> > > > selinux_set_mapping() from an AVC callback, since
> > > > selinux_set_mapping()
> > > > itself calls avc_reset() to flush any cache entries before
> > > > reloading
> > > > the mapping, so that would produce infinite
> > > > recursion.  Sigh.  Could
> > > > change selinux_set_mapping() to not do that in libselinux, but that
> > > > won't help on existing releases.  Maybe we should just convert it
> > > > over
> > > > to using selinux_check_access() and drop all direct usage of the
> > > > AVC.
> > > > 
> > > > > > > 
> > > > > > > smc_socket was added by the kernel developers as part of the
> > > > > > > merge
> > > > > > > with
> > > > > > > net-next since we now trigger a build failure in the kernel
> > > > > > > if
> > > > > > > any
> > > > > > > new
> > > > > > > address families are introduced without adding a
> > > > > > > corresponding
> > > > > > > security
> > > > > > > class (so that SELinux always supports a separate class per
> > > > > > > network
> > > > > > > address family going forward). So there have been no policy
> > > > > > > patches
> > > > > > > submitted yet to define it in refpolicy even AFAIK.
> > > > > > > 
> > > > > > > [1] https://github.com/SELinuxProject/selinux/issues/34
> > > > > 
> > > > > So i suppose its an unordered class? or do i have to order this
> > > > > one
> > > > > to avoid future issues?
> > > > > 
> > > > > > 
> > > > > > BTW, I'm not sure what you did to trigger the problem.  When I
> > > > > > tested
> > > > > > the extended socket classes, I added them to my running policy
> > > > > > via
> > > > > > a
> > > > > > CIL module like this:
> > > > > 
> > > > > Well i just added this commit (ignore the typo in the name) which
> > > > > adds the new class to the unordered socket list
> > > > > 
> > > > > https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659
> > > > > f347
> > > > > 863f09ec9caeceb56
> > > > > 
> > > > > then i built rpms using this script:
> > > > > 
> > > > > https://github.com/DefenSec/dssp2-standard/blob/master/support/rp
> > > > > m/ds
> > > > > sp2-standard.sh
> > > > > 
> > > > > and just rpm -Uvh the new rpm
> > > > > 
> > > > > > 
> > > > > > (policycap extended_socket_class)
> > > > > > 
> > > > > > (classcommon sctp_socket socket)
> > > > > > (class sctp_socket (node_bind))
> > > > > > 
> > > > > > <snip>
> > > > > > 
> > > > > > (classcommon qipcrtr_socket socket)
> > > > > > (class qipcrtr_socket ())
> > > > > > 
> > > > > > (classorder (unordered sctp_socket icmp_socket ax25_socket
> > > > > > ipx_socket
> > > > > > netrom_socket bridge_socket atmpvc_socket x25_socket
> > > > > > rose_socket
> > > > > > decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
> > > > > > llc_socket ib_socket mpls_socket can_socket tipc_socket
> > > > > > bluetooth_socket iucv_socket rxrpc_socket isdn_socket
> > > > > > phonet_socket
> > > > > > ieee802154_socket caif_socket alg_socket nfc_socket
> > > > > > vsock_socket
> > > > > > kcm_socket qipcrtr_socket))
> > > > > > 
> > > > > > The classorder statement at the end ensured that they were
> > > > > > appended
> > > > > > to
> > > > > > the end of the class list and therefore did not break anything.
> > > > > > 
> > > > 
> > > > Looks like you failed to include a classorder statement for it as I
> > > > did
> > > > above.  That's still required to avoid breaking legacy userspace
> > > > object
> > > > managers.
> > > > 
> > > > 
> > > 
> > > No I added "scm_socket" (i renamed it later to smc_socket) to my list
> > > or unordered classorder:
> > > 
> > > ;
> > > ; Class order of unordered Linux access vectors
> > > ;
> > > 
> > > (classorder
> > >     (unordered
> > >         alg_socket
> > >         atmpvc_socket
> > >         atmsvc_socket
> > >         ax25_socket
> > >         bluetooth_socket
> > >         caif_socket
> > >         can_socket
> > >         decnet_socket
> > >         icmp_socket
> > >         ieee802154_socket
> > >         ipx_socket
> > >         irda_socket
> > >         isdn_socket
> > >         iucv_socket
> > >         kcm_socket
> > >         llc_socket
> > >         netrom_socket
> > >         nfc_socket
> > >         phonet_socket
> > >         pppox_socket
> > >         qipcrtr_socket
> > >         rds_socket
> > >         rose_socket
> > >         rxrpc_socket
> > >         scm_socket
> > >         sctp_socket
> > >         tipc_socket
> > >         vsock_socket
> > >         x25_socket
> > >     )
> > > )
> > 
> > Then I'm confused as to why you encountered breakage.  Doesn't CIL
> > ensure that all of those classes are appended to the existing class
> > list?  In which case it won't disturb the dbus class definitions.
> > 

If I would have left out the classorder entry , then a reboot would - i suppose - not have fixed the issue

> 
> CIL appends unordered classes to the existing class list, so I am not sure
> what is going on.
> 
> Jim
> 
> 
> 
> -- 
> James Carter <jwcart2@tycho.nsa.gov>
> National Security Agency

-- 
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] 14+ messages in thread

* Re: userspace object manager confused
  2017-03-31 14:17               ` Dominick Grift
@ 2017-03-31 14:30                 ` James Carter
  2017-03-31 14:39                   ` Dominick Grift
  0 siblings, 1 reply; 14+ messages in thread
From: James Carter @ 2017-03-31 14:30 UTC (permalink / raw)
  To: selinux

On 03/31/2017 10:17 AM, Dominick Grift wrote:
> On Fri, Mar 31, 2017 at 10:12:50AM -0400, James Carter wrote:
>> On 03/31/2017 10:10 AM, Stephen Smalley wrote:
>>> On Fri, 2017-03-31 at 15:59 +0200, Dominick Grift wrote:
>>>> On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
>>>>> On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
>>>>>> On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
>>>>>>> On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
>>>>>>>> On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
>>>>>>>>> I vaguely recall that we discussed this issue or at least
>>>>>>>>> that
>>>>>>>>> i
>>>>>>>>> mentioned it here but i can't recall the outcome if any:
>>>>>>>>>
>>>>>>>>> So today on my rawhide system i noticed that i somehow
>>>>>>>>> forgot
>>>>>>>>> to
>>>>>>>>> add
>>>>>>>>> support for the smc_socket class (i suspect that is part of
>>>>>>>>> the
>>>>>>>>> extended socket class patches)
>>>>>>>>>
>>>>>>>>> I added the class (which i suppose is unordered like the
>>>>>>>>> othe
>>>>>>>>> extended socket classes) but as soon as I loaded up the
>>>>>>>>> policy
>>>>>>>>> with
>>>>>>>>> the new unordered smc_socket class the system became
>>>>>>>>> unusable.
>>>>>>>>>
>>>>>>>>> This is because the dbus object manager became confused due
>>>>>>>>> to
>>>>>>>>> my
>>>>>>>>> adding a new (unordered) class at runtime, and that the
>>>>>>>>> dbus
>>>>>>>>> class
>>>>>>>>> was no longer working.
>>>>>>>>>
>>>>>>>>> Modern systems heavily rely on dbus at the heart and so it
>>>>>>>>> is
>>>>>>>>> undesire-able that this happens.
>>>>>>>>>
>>>>>>>>> A reboot clears this issue up but adding (unordered)
>>>>>>>>> classes at
>>>>>>>>> runtime should not cause these issues i suspect
>>>>>>>>
>>>>>>>> dbusd doesn't use selinux_check_access() and therefore does
>>>>>>>> not
>>>>>>>> yet
>>>>>>>> support reordering of their classes/permissions at
>>>>>>>> runtime.  The
>>>>>>>> same
>>>>>>>> is true of all userspace object managers created before
>>>>>>>> selinux_check_access() was introduced - anything that
>>>>>>>> directly
>>>>>>>> calls
>>>>>>>> security_compute_av() or avc_has_perm(). dbusd does call
>>>>>>>> selinux_set_mapping() at startup, so it can correctly handle
>>>>>>>> reordering of classes/permissions across restarts, but not
>>>>>>>> while
>>>>>>>> it
>>>>>>>> is
>>>>>>>> running.  Calling selinux_set_mapping() again upon policy
>>>>>>>> reloads
>>>>>>>> (e.g.
>>>>>>>> from policy_reload_callback() if (event ==
>>>>>>>> AVC_CALLBACK_RESET)
>>>>>>>> before
>>>>>>>> returning) may fix this problem, but requires proper
>>>>>>>> locking.  Even
>>>>>>>> better would be to rid the dbusd selinux implementation of
>>>>>>>> threading
>>>>>>>> entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92
>>>>>>>> 831#
>>>>>>>> c4
>>>>>>
>>>>>> Thank you, I suppose i should take this to them then.
>>>>>
>>>>> No, someone who uses SELinux will need to develop a patch and test
>>>>> it
>>>>> for them; dbusd maintainer doesn't use SELinux himself.
>>>>>
>>>>> Looks like I'm also wrong about being able to call
>>>>> selinux_set_mapping() from an AVC callback, since
>>>>> selinux_set_mapping()
>>>>> itself calls avc_reset() to flush any cache entries before
>>>>> reloading
>>>>> the mapping, so that would produce infinite
>>>>> recursion.  Sigh.  Could
>>>>> change selinux_set_mapping() to not do that in libselinux, but that
>>>>> won't help on existing releases.  Maybe we should just convert it
>>>>> over
>>>>> to using selinux_check_access() and drop all direct usage of the
>>>>> AVC.
>>>>>
>>>>>>>>
>>>>>>>> smc_socket was added by the kernel developers as part of the
>>>>>>>> merge
>>>>>>>> with
>>>>>>>> net-next since we now trigger a build failure in the kernel
>>>>>>>> if
>>>>>>>> any
>>>>>>>> new
>>>>>>>> address families are introduced without adding a
>>>>>>>> corresponding
>>>>>>>> security
>>>>>>>> class (so that SELinux always supports a separate class per
>>>>>>>> network
>>>>>>>> address family going forward). So there have been no policy
>>>>>>>> patches
>>>>>>>> submitted yet to define it in refpolicy even AFAIK.
>>>>>>>>
>>>>>>>> [1] https://github.com/SELinuxProject/selinux/issues/34
>>>>>>
>>>>>> So i suppose its an unordered class? or do i have to order this
>>>>>> one
>>>>>> to avoid future issues?
>>>>>>
>>>>>>>
>>>>>>> BTW, I'm not sure what you did to trigger the problem.  When I
>>>>>>> tested
>>>>>>> the extended socket classes, I added them to my running policy
>>>>>>> via
>>>>>>> a
>>>>>>> CIL module like this:
>>>>>>
>>>>>> Well i just added this commit (ignore the typo in the name) which
>>>>>> adds the new class to the unordered socket list
>>>>>>
>>>>>> https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659
>>>>>> f347
>>>>>> 863f09ec9caeceb56
>>>>>>
>>>>>> then i built rpms using this script:
>>>>>>
>>>>>> https://github.com/DefenSec/dssp2-standard/blob/master/support/rp
>>>>>> m/ds
>>>>>> sp2-standard.sh
>>>>>>
>>>>>> and just rpm -Uvh the new rpm
>>>>>>
>>>>>>>
>>>>>>> (policycap extended_socket_class)
>>>>>>>
>>>>>>> (classcommon sctp_socket socket)
>>>>>>> (class sctp_socket (node_bind))
>>>>>>>
>>>>>>> <snip>
>>>>>>>
>>>>>>> (classcommon qipcrtr_socket socket)
>>>>>>> (class qipcrtr_socket ())
>>>>>>>
>>>>>>> (classorder (unordered sctp_socket icmp_socket ax25_socket
>>>>>>> ipx_socket
>>>>>>> netrom_socket bridge_socket atmpvc_socket x25_socket
>>>>>>> rose_socket
>>>>>>> decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
>>>>>>> llc_socket ib_socket mpls_socket can_socket tipc_socket
>>>>>>> bluetooth_socket iucv_socket rxrpc_socket isdn_socket
>>>>>>> phonet_socket
>>>>>>> ieee802154_socket caif_socket alg_socket nfc_socket
>>>>>>> vsock_socket
>>>>>>> kcm_socket qipcrtr_socket))
>>>>>>>
>>>>>>> The classorder statement at the end ensured that they were
>>>>>>> appended
>>>>>>> to
>>>>>>> the end of the class list and therefore did not break anything.
>>>>>>>
>>>>>
>>>>> Looks like you failed to include a classorder statement for it as I
>>>>> did
>>>>> above.  That's still required to avoid breaking legacy userspace
>>>>> object
>>>>> managers.
>>>>>
>>>>>
>>>>
>>>> No I added "scm_socket" (i renamed it later to smc_socket) to my list
>>>> or unordered classorder:
>>>>
>>>> ;
>>>> ; Class order of unordered Linux access vectors
>>>> ;
>>>>
>>>> (classorder
>>>>     (unordered
>>>>         alg_socket
>>>>         atmpvc_socket
>>>>         atmsvc_socket
>>>>         ax25_socket
>>>>         bluetooth_socket
>>>>         caif_socket
>>>>         can_socket
>>>>         decnet_socket
>>>>         icmp_socket
>>>>         ieee802154_socket
>>>>         ipx_socket
>>>>         irda_socket
>>>>         isdn_socket
>>>>         iucv_socket
>>>>         kcm_socket
>>>>         llc_socket
>>>>         netrom_socket
>>>>         nfc_socket
>>>>         phonet_socket
>>>>         pppox_socket
>>>>         qipcrtr_socket
>>>>         rds_socket
>>>>         rose_socket
>>>>         rxrpc_socket
>>>>         scm_socket
>>>>         sctp_socket
>>>>         tipc_socket
>>>>         vsock_socket
>>>>         x25_socket
>>>>     )
>>>> )
>>>

You are reordering classes here since you didn't put scm_socket at the end of 
the list. CIL just adds all of the unordered classes in the order that it parses 
them to the end of the ordered list. So could it be that sctp_socket, 
tipc_socket, vsock_socket, x25_socket changing caused the problem?

Jim

>>> Then I'm confused as to why you encountered breakage.  Doesn't CIL
>>> ensure that all of those classes are appended to the existing class
>>> list?  In which case it won't disturb the dbus class definitions.
>>>
>
> If I would have left out the classorder entry , then a reboot would - i suppose - not have fixed the issue
>
>>
>> CIL appends unordered classes to the existing class list, so I am not sure
>> what is going on.
>>
>> Jim
>>
>>
>>
>> --
>> James Carter <jwcart2@tycho.nsa.gov>
>> National Security Agency
>
>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>


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

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

* Re: userspace object manager confused
  2017-03-31 14:12             ` James Carter
  2017-03-31 14:17               ` Dominick Grift
@ 2017-03-31 14:32               ` Dominick Grift
  1 sibling, 0 replies; 14+ messages in thread
From: Dominick Grift @ 2017-03-31 14:32 UTC (permalink / raw)
  To: selinux

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

On Fri, Mar 31, 2017 at 10:12:50AM -0400, James Carter wrote:
> On 03/31/2017 10:10 AM, Stephen Smalley wrote:
> > On Fri, 2017-03-31 at 15:59 +0200, Dominick Grift wrote:
> > > On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
> > > > On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
> > > > > On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
> > > > > > On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> > > > > > > On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > > > > > > > I vaguely recall that we discussed this issue or at least
> > > > > > > > that
> > > > > > > > i
> > > > > > > > mentioned it here but i can't recall the outcome if any:
> > > > > > > > 
> > > > > > > > So today on my rawhide system i noticed that i somehow
> > > > > > > > forgot
> > > > > > > > to
> > > > > > > > add
> > > > > > > > support for the smc_socket class (i suspect that is part of
> > > > > > > > the
> > > > > > > > extended socket class patches)
> > > > > > > > 
> > > > > > > > I added the class (which i suppose is unordered like the
> > > > > > > > othe
> > > > > > > > extended socket classes) but as soon as I loaded up the
> > > > > > > > policy
> > > > > > > > with
> > > > > > > > the new unordered smc_socket class the system became
> > > > > > > > unusable.
> > > > > > > > 
> > > > > > > > This is because the dbus object manager became confused due
> > > > > > > > to
> > > > > > > > my
> > > > > > > > adding a new (unordered) class at runtime, and that the
> > > > > > > > dbus
> > > > > > > > class
> > > > > > > > was no longer working.
> > > > > > > > 
> > > > > > > > Modern systems heavily rely on dbus at the heart and so it
> > > > > > > > is
> > > > > > > > undesire-able that this happens.
> > > > > > > > 
> > > > > > > > A reboot clears this issue up but adding (unordered)
> > > > > > > > classes at
> > > > > > > > runtime should not cause these issues i suspect
> > > > > > > 
> > > > > > > dbusd doesn't use selinux_check_access() and therefore does
> > > > > > > not
> > > > > > > yet
> > > > > > > support reordering of their classes/permissions at
> > > > > > > runtime.  The
> > > > > > > same
> > > > > > > is true of all userspace object managers created before
> > > > > > > selinux_check_access() was introduced - anything that
> > > > > > > directly
> > > > > > > calls
> > > > > > > security_compute_av() or avc_has_perm(). dbusd does call
> > > > > > > selinux_set_mapping() at startup, so it can correctly handle
> > > > > > > reordering of classes/permissions across restarts, but not
> > > > > > > while
> > > > > > > it
> > > > > > > is
> > > > > > > running.  Calling selinux_set_mapping() again upon policy
> > > > > > > reloads
> > > > > > > (e.g.
> > > > > > > from policy_reload_callback() if (event ==
> > > > > > > AVC_CALLBACK_RESET)
> > > > > > > before
> > > > > > > returning) may fix this problem, but requires proper
> > > > > > > locking.  Even
> > > > > > > better would be to rid the dbusd selinux implementation of
> > > > > > > threading
> > > > > > > entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92
> > > > > > > 831#
> > > > > > > c4
> > > > > 
> > > > > Thank you, I suppose i should take this to them then.
> > > > 
> > > > No, someone who uses SELinux will need to develop a patch and test
> > > > it
> > > > for them; dbusd maintainer doesn't use SELinux himself.
> > > > 
> > > > Looks like I'm also wrong about being able to call
> > > > selinux_set_mapping() from an AVC callback, since
> > > > selinux_set_mapping()
> > > > itself calls avc_reset() to flush any cache entries before
> > > > reloading
> > > > the mapping, so that would produce infinite
> > > > recursion.  Sigh.  Could
> > > > change selinux_set_mapping() to not do that in libselinux, but that
> > > > won't help on existing releases.  Maybe we should just convert it
> > > > over
> > > > to using selinux_check_access() and drop all direct usage of the
> > > > AVC.
> > > > 
> > > > > > > 
> > > > > > > smc_socket was added by the kernel developers as part of the
> > > > > > > merge
> > > > > > > with
> > > > > > > net-next since we now trigger a build failure in the kernel
> > > > > > > if
> > > > > > > any
> > > > > > > new
> > > > > > > address families are introduced without adding a
> > > > > > > corresponding
> > > > > > > security
> > > > > > > class (so that SELinux always supports a separate class per
> > > > > > > network
> > > > > > > address family going forward). So there have been no policy
> > > > > > > patches
> > > > > > > submitted yet to define it in refpolicy even AFAIK.
> > > > > > > 
> > > > > > > [1] https://github.com/SELinuxProject/selinux/issues/34
> > > > > 
> > > > > So i suppose its an unordered class? or do i have to order this
> > > > > one
> > > > > to avoid future issues?
> > > > > 
> > > > > > 
> > > > > > BTW, I'm not sure what you did to trigger the problem.  When I
> > > > > > tested
> > > > > > the extended socket classes, I added them to my running policy
> > > > > > via
> > > > > > a
> > > > > > CIL module like this:
> > > > > 
> > > > > Well i just added this commit (ignore the typo in the name) which
> > > > > adds the new class to the unordered socket list
> > > > > 
> > > > > https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659
> > > > > f347
> > > > > 863f09ec9caeceb56
> > > > > 
> > > > > then i built rpms using this script:
> > > > > 
> > > > > https://github.com/DefenSec/dssp2-standard/blob/master/support/rp
> > > > > m/ds
> > > > > sp2-standard.sh
> > > > > 
> > > > > and just rpm -Uvh the new rpm
> > > > > 
> > > > > > 
> > > > > > (policycap extended_socket_class)
> > > > > > 
> > > > > > (classcommon sctp_socket socket)
> > > > > > (class sctp_socket (node_bind))
> > > > > > 
> > > > > > <snip>
> > > > > > 
> > > > > > (classcommon qipcrtr_socket socket)
> > > > > > (class qipcrtr_socket ())
> > > > > > 
> > > > > > (classorder (unordered sctp_socket icmp_socket ax25_socket
> > > > > > ipx_socket
> > > > > > netrom_socket bridge_socket atmpvc_socket x25_socket
> > > > > > rose_socket
> > > > > > decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
> > > > > > llc_socket ib_socket mpls_socket can_socket tipc_socket
> > > > > > bluetooth_socket iucv_socket rxrpc_socket isdn_socket
> > > > > > phonet_socket
> > > > > > ieee802154_socket caif_socket alg_socket nfc_socket
> > > > > > vsock_socket
> > > > > > kcm_socket qipcrtr_socket))
> > > > > > 
> > > > > > The classorder statement at the end ensured that they were
> > > > > > appended
> > > > > > to
> > > > > > the end of the class list and therefore did not break anything.
> > > > > > 
> > > > 
> > > > Looks like you failed to include a classorder statement for it as I
> > > > did
> > > > above.  That's still required to avoid breaking legacy userspace
> > > > object
> > > > managers.
> > > > 
> > > > 
> > > 
> > > No I added "scm_socket" (i renamed it later to smc_socket) to my list
> > > or unordered classorder:
> > > 
> > > ;
> > > ; Class order of unordered Linux access vectors
> > > ;
> > > 
> > > (classorder
> > >     (unordered
> > >         alg_socket
> > >         atmpvc_socket
> > >         atmsvc_socket
> > >         ax25_socket
> > >         bluetooth_socket
> > >         caif_socket
> > >         can_socket
> > >         decnet_socket
> > >         icmp_socket
> > >         ieee802154_socket
> > >         ipx_socket
> > >         irda_socket
> > >         isdn_socket
> > >         iucv_socket
> > >         kcm_socket
> > >         llc_socket
> > >         netrom_socket
> > >         nfc_socket
> > >         phonet_socket
> > >         pppox_socket
> > >         qipcrtr_socket
> > >         rds_socket
> > >         rose_socket
> > >         rxrpc_socket
> > >         scm_socket
> > >         sctp_socket
> > >         tipc_socket
> > >         vsock_socket
> > >         x25_socket
> > >     )
> > > )
> > 
> > Then I'm confused as to why you encountered breakage.  Doesn't CIL
> > ensure that all of those classes are appended to the existing class
> > list?  In which case it won't disturb the dbus class definitions.
> > 
> 
> CIL appends unordered classes to the existing class list, so I am not sure
> what is going on.

Here is my theory:

I have various (classorder (unordered)) lists spread throughout my policy (access_vectors,cil dbus.cil systemd.cil mcstrans,cil pam.cil)

This is because i differentiate between user space and linux access vectors: I want vendors to be able to add modules for user space components with , if applicable, their respective access vectors)

I create myapp, made it an object manager, now i want to provide a module that included support for the object manager by declaring my user space access vector

My linux access vector classes are classordered in access_vectors.cil, however:

the dbus class is classordered in dbus.cil -- this is because the dbus class is tied to the dbus module: i.e. if i de-install dbus.cil then the dbus access
vector will be removed.

So with that: could it be that we still have ordering issues (ironically)

because theres various (classorder (unordered)) lists and they just get appended: so if it first processes the one in access_vectors, and then the one in dbus.cil, then it will still mess up the order

should in not instead first collect all the (classorder (unonrdered)) lists throughout the policy, thrown them on one heap and and then process them?

hmm ... that wouldnt work either i suppose....

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

-- 
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] 14+ messages in thread

* Re: userspace object manager confused
  2017-03-31 14:30                 ` James Carter
@ 2017-03-31 14:39                   ` Dominick Grift
  2017-03-31 14:53                     ` James Carter
  0 siblings, 1 reply; 14+ messages in thread
From: Dominick Grift @ 2017-03-31 14:39 UTC (permalink / raw)
  To: selinux

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

On Fri, Mar 31, 2017 at 10:30:22AM -0400, James Carter wrote:
> On 03/31/2017 10:17 AM, Dominick Grift wrote:
> > On Fri, Mar 31, 2017 at 10:12:50AM -0400, James Carter wrote:
> > > On 03/31/2017 10:10 AM, Stephen Smalley wrote:
> > > > On Fri, 2017-03-31 at 15:59 +0200, Dominick Grift wrote:
> > > > > On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
> > > > > > On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
> > > > > > > On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
> > > > > > > > On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> > > > > > > > > On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > > > > > > > > > I vaguely recall that we discussed this issue or at least
> > > > > > > > > > that
> > > > > > > > > > i
> > > > > > > > > > mentioned it here but i can't recall the outcome if any:
> > > > > > > > > > 
> > > > > > > > > > So today on my rawhide system i noticed that i somehow
> > > > > > > > > > forgot
> > > > > > > > > > to
> > > > > > > > > > add
> > > > > > > > > > support for the smc_socket class (i suspect that is part of
> > > > > > > > > > the
> > > > > > > > > > extended socket class patches)
> > > > > > > > > > 
> > > > > > > > > > I added the class (which i suppose is unordered like the
> > > > > > > > > > othe
> > > > > > > > > > extended socket classes) but as soon as I loaded up the
> > > > > > > > > > policy
> > > > > > > > > > with
> > > > > > > > > > the new unordered smc_socket class the system became
> > > > > > > > > > unusable.
> > > > > > > > > > 
> > > > > > > > > > This is because the dbus object manager became confused due
> > > > > > > > > > to
> > > > > > > > > > my
> > > > > > > > > > adding a new (unordered) class at runtime, and that the
> > > > > > > > > > dbus
> > > > > > > > > > class
> > > > > > > > > > was no longer working.
> > > > > > > > > > 
> > > > > > > > > > Modern systems heavily rely on dbus at the heart and so it
> > > > > > > > > > is
> > > > > > > > > > undesire-able that this happens.
> > > > > > > > > > 
> > > > > > > > > > A reboot clears this issue up but adding (unordered)
> > > > > > > > > > classes at
> > > > > > > > > > runtime should not cause these issues i suspect
> > > > > > > > > 
> > > > > > > > > dbusd doesn't use selinux_check_access() and therefore does
> > > > > > > > > not
> > > > > > > > > yet
> > > > > > > > > support reordering of their classes/permissions at
> > > > > > > > > runtime.  The
> > > > > > > > > same
> > > > > > > > > is true of all userspace object managers created before
> > > > > > > > > selinux_check_access() was introduced - anything that
> > > > > > > > > directly
> > > > > > > > > calls
> > > > > > > > > security_compute_av() or avc_has_perm(). dbusd does call
> > > > > > > > > selinux_set_mapping() at startup, so it can correctly handle
> > > > > > > > > reordering of classes/permissions across restarts, but not
> > > > > > > > > while
> > > > > > > > > it
> > > > > > > > > is
> > > > > > > > > running.  Calling selinux_set_mapping() again upon policy
> > > > > > > > > reloads
> > > > > > > > > (e.g.
> > > > > > > > > from policy_reload_callback() if (event ==
> > > > > > > > > AVC_CALLBACK_RESET)
> > > > > > > > > before
> > > > > > > > > returning) may fix this problem, but requires proper
> > > > > > > > > locking.  Even
> > > > > > > > > better would be to rid the dbusd selinux implementation of
> > > > > > > > > threading
> > > > > > > > > entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92
> > > > > > > > > 831#
> > > > > > > > > c4
> > > > > > > 
> > > > > > > Thank you, I suppose i should take this to them then.
> > > > > > 
> > > > > > No, someone who uses SELinux will need to develop a patch and test
> > > > > > it
> > > > > > for them; dbusd maintainer doesn't use SELinux himself.
> > > > > > 
> > > > > > Looks like I'm also wrong about being able to call
> > > > > > selinux_set_mapping() from an AVC callback, since
> > > > > > selinux_set_mapping()
> > > > > > itself calls avc_reset() to flush any cache entries before
> > > > > > reloading
> > > > > > the mapping, so that would produce infinite
> > > > > > recursion.  Sigh.  Could
> > > > > > change selinux_set_mapping() to not do that in libselinux, but that
> > > > > > won't help on existing releases.  Maybe we should just convert it
> > > > > > over
> > > > > > to using selinux_check_access() and drop all direct usage of the
> > > > > > AVC.
> > > > > > 
> > > > > > > > > 
> > > > > > > > > smc_socket was added by the kernel developers as part of the
> > > > > > > > > merge
> > > > > > > > > with
> > > > > > > > > net-next since we now trigger a build failure in the kernel
> > > > > > > > > if
> > > > > > > > > any
> > > > > > > > > new
> > > > > > > > > address families are introduced without adding a
> > > > > > > > > corresponding
> > > > > > > > > security
> > > > > > > > > class (so that SELinux always supports a separate class per
> > > > > > > > > network
> > > > > > > > > address family going forward). So there have been no policy
> > > > > > > > > patches
> > > > > > > > > submitted yet to define it in refpolicy even AFAIK.
> > > > > > > > > 
> > > > > > > > > [1] https://github.com/SELinuxProject/selinux/issues/34
> > > > > > > 
> > > > > > > So i suppose its an unordered class? or do i have to order this
> > > > > > > one
> > > > > > > to avoid future issues?
> > > > > > > 
> > > > > > > > 
> > > > > > > > BTW, I'm not sure what you did to trigger the problem.  When I
> > > > > > > > tested
> > > > > > > > the extended socket classes, I added them to my running policy
> > > > > > > > via
> > > > > > > > a
> > > > > > > > CIL module like this:
> > > > > > > 
> > > > > > > Well i just added this commit (ignore the typo in the name) which
> > > > > > > adds the new class to the unordered socket list
> > > > > > > 
> > > > > > > https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659
> > > > > > > f347
> > > > > > > 863f09ec9caeceb56
> > > > > > > 
> > > > > > > then i built rpms using this script:
> > > > > > > 
> > > > > > > https://github.com/DefenSec/dssp2-standard/blob/master/support/rp
> > > > > > > m/ds
> > > > > > > sp2-standard.sh
> > > > > > > 
> > > > > > > and just rpm -Uvh the new rpm
> > > > > > > 
> > > > > > > > 
> > > > > > > > (policycap extended_socket_class)
> > > > > > > > 
> > > > > > > > (classcommon sctp_socket socket)
> > > > > > > > (class sctp_socket (node_bind))
> > > > > > > > 
> > > > > > > > <snip>
> > > > > > > > 
> > > > > > > > (classcommon qipcrtr_socket socket)
> > > > > > > > (class qipcrtr_socket ())
> > > > > > > > 
> > > > > > > > (classorder (unordered sctp_socket icmp_socket ax25_socket
> > > > > > > > ipx_socket
> > > > > > > > netrom_socket bridge_socket atmpvc_socket x25_socket
> > > > > > > > rose_socket
> > > > > > > > decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
> > > > > > > > llc_socket ib_socket mpls_socket can_socket tipc_socket
> > > > > > > > bluetooth_socket iucv_socket rxrpc_socket isdn_socket
> > > > > > > > phonet_socket
> > > > > > > > ieee802154_socket caif_socket alg_socket nfc_socket
> > > > > > > > vsock_socket
> > > > > > > > kcm_socket qipcrtr_socket))
> > > > > > > > 
> > > > > > > > The classorder statement at the end ensured that they were
> > > > > > > > appended
> > > > > > > > to
> > > > > > > > the end of the class list and therefore did not break anything.
> > > > > > > > 
> > > > > > 
> > > > > > Looks like you failed to include a classorder statement for it as I
> > > > > > did
> > > > > > above.  That's still required to avoid breaking legacy userspace
> > > > > > object
> > > > > > managers.
> > > > > > 
> > > > > > 
> > > > > 
> > > > > No I added "scm_socket" (i renamed it later to smc_socket) to my list
> > > > > or unordered classorder:
> > > > > 
> > > > > ;
> > > > > ; Class order of unordered Linux access vectors
> > > > > ;
> > > > > 
> > > > > (classorder
> > > > >     (unordered
> > > > >         alg_socket
> > > > >         atmpvc_socket
> > > > >         atmsvc_socket
> > > > >         ax25_socket
> > > > >         bluetooth_socket
> > > > >         caif_socket
> > > > >         can_socket
> > > > >         decnet_socket
> > > > >         icmp_socket
> > > > >         ieee802154_socket
> > > > >         ipx_socket
> > > > >         irda_socket
> > > > >         isdn_socket
> > > > >         iucv_socket
> > > > >         kcm_socket
> > > > >         llc_socket
> > > > >         netrom_socket
> > > > >         nfc_socket
> > > > >         phonet_socket
> > > > >         pppox_socket
> > > > >         qipcrtr_socket
> > > > >         rds_socket
> > > > >         rose_socket
> > > > >         rxrpc_socket
> > > > >         scm_socket
> > > > >         sctp_socket
> > > > >         tipc_socket
> > > > >         vsock_socket
> > > > >         x25_socket
> > > > >     )
> > > > > )
> > > > 
> 
> You are reordering classes here since you didn't put scm_socket at the end
> of the list. CIL just adds all of the unordered classes in the order that it
> parses them to the end of the ordered list. So could it be that sctp_socket,
> tipc_socket, vsock_socket, x25_socket changing caused the problem?
> 
> Jim
> 
> > > > Then I'm confused as to why you encountered breakage.  Doesn't CIL
> > > > ensure that all of those classes are appended to the existing class
> > > > list?  In which case it won't disturb the dbus class definitions.
> > > > 
> > 
> > If I would have left out the classorder entry , then a reboot would - i suppose - not have fixed the issue
> > 
> > > 
> > > CIL appends unordered classes to the existing class list, so I am not sure
> > > what is going on.
> > > 
> > > Jim

What was the meaning of "unordered" if order matters just as much as "ordered"?

dbus class is unordered, smc_socket is unordered. how can these two unordered classes cause ordering issues?

> > > 
> > > 
> > > 
> > > --
> > > James Carter <jwcart2@tycho.nsa.gov>
> > > National Security Agency
> > 
> > 
> > 
> > _______________________________________________
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
> > 
> 
> 
> -- 
> James Carter <jwcart2@tycho.nsa.gov>
> National Security Agency
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.

-- 
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] 14+ messages in thread

* Re: userspace object manager confused
  2017-03-31 14:39                   ` Dominick Grift
@ 2017-03-31 14:53                     ` James Carter
  2017-03-31 15:01                       ` Dominick Grift
  0 siblings, 1 reply; 14+ messages in thread
From: James Carter @ 2017-03-31 14:53 UTC (permalink / raw)
  To: selinux

On 03/31/2017 10:39 AM, Dominick Grift wrote:
> On Fri, Mar 31, 2017 at 10:30:22AM -0400, James Carter wrote:
>> On 03/31/2017 10:17 AM, Dominick Grift wrote:
>>> On Fri, Mar 31, 2017 at 10:12:50AM -0400, James Carter wrote:
>>>> On 03/31/2017 10:10 AM, Stephen Smalley wrote:
>>>>> On Fri, 2017-03-31 at 15:59 +0200, Dominick Grift wrote:
>>>>>> On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
>>>>>>> On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
>>>>>>>> On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
>>>>>>>>> On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
>>>>>>>>>> On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
>>>>>>>>>>> I vaguely recall that we discussed this issue or at least
>>>>>>>>>>> that
>>>>>>>>>>> i
>>>>>>>>>>> mentioned it here but i can't recall the outcome if any:
>>>>>>>>>>>
>>>>>>>>>>> So today on my rawhide system i noticed that i somehow
>>>>>>>>>>> forgot
>>>>>>>>>>> to
>>>>>>>>>>> add
>>>>>>>>>>> support for the smc_socket class (i suspect that is part of
>>>>>>>>>>> the
>>>>>>>>>>> extended socket class patches)
>>>>>>>>>>>
>>>>>>>>>>> I added the class (which i suppose is unordered like the
>>>>>>>>>>> othe
>>>>>>>>>>> extended socket classes) but as soon as I loaded up the
>>>>>>>>>>> policy
>>>>>>>>>>> with
>>>>>>>>>>> the new unordered smc_socket class the system became
>>>>>>>>>>> unusable.
>>>>>>>>>>>
>>>>>>>>>>> This is because the dbus object manager became confused due
>>>>>>>>>>> to
>>>>>>>>>>> my
>>>>>>>>>>> adding a new (unordered) class at runtime, and that the
>>>>>>>>>>> dbus
>>>>>>>>>>> class
>>>>>>>>>>> was no longer working.
>>>>>>>>>>>
>>>>>>>>>>> Modern systems heavily rely on dbus at the heart and so it
>>>>>>>>>>> is
>>>>>>>>>>> undesire-able that this happens.
>>>>>>>>>>>
>>>>>>>>>>> A reboot clears this issue up but adding (unordered)
>>>>>>>>>>> classes at
>>>>>>>>>>> runtime should not cause these issues i suspect
>>>>>>>>>>
>>>>>>>>>> dbusd doesn't use selinux_check_access() and therefore does
>>>>>>>>>> not
>>>>>>>>>> yet
>>>>>>>>>> support reordering of their classes/permissions at
>>>>>>>>>> runtime.  The
>>>>>>>>>> same
>>>>>>>>>> is true of all userspace object managers created before
>>>>>>>>>> selinux_check_access() was introduced - anything that
>>>>>>>>>> directly
>>>>>>>>>> calls
>>>>>>>>>> security_compute_av() or avc_has_perm(). dbusd does call
>>>>>>>>>> selinux_set_mapping() at startup, so it can correctly handle
>>>>>>>>>> reordering of classes/permissions across restarts, but not
>>>>>>>>>> while
>>>>>>>>>> it
>>>>>>>>>> is
>>>>>>>>>> running.  Calling selinux_set_mapping() again upon policy
>>>>>>>>>> reloads
>>>>>>>>>> (e.g.
>>>>>>>>>> from policy_reload_callback() if (event ==
>>>>>>>>>> AVC_CALLBACK_RESET)
>>>>>>>>>> before
>>>>>>>>>> returning) may fix this problem, but requires proper
>>>>>>>>>> locking.  Even
>>>>>>>>>> better would be to rid the dbusd selinux implementation of
>>>>>>>>>> threading
>>>>>>>>>> entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92
>>>>>>>>>> 831#
>>>>>>>>>> c4
>>>>>>>>
>>>>>>>> Thank you, I suppose i should take this to them then.
>>>>>>>
>>>>>>> No, someone who uses SELinux will need to develop a patch and test
>>>>>>> it
>>>>>>> for them; dbusd maintainer doesn't use SELinux himself.
>>>>>>>
>>>>>>> Looks like I'm also wrong about being able to call
>>>>>>> selinux_set_mapping() from an AVC callback, since
>>>>>>> selinux_set_mapping()
>>>>>>> itself calls avc_reset() to flush any cache entries before
>>>>>>> reloading
>>>>>>> the mapping, so that would produce infinite
>>>>>>> recursion.  Sigh.  Could
>>>>>>> change selinux_set_mapping() to not do that in libselinux, but that
>>>>>>> won't help on existing releases.  Maybe we should just convert it
>>>>>>> over
>>>>>>> to using selinux_check_access() and drop all direct usage of the
>>>>>>> AVC.
>>>>>>>
>>>>>>>>>>
>>>>>>>>>> smc_socket was added by the kernel developers as part of the
>>>>>>>>>> merge
>>>>>>>>>> with
>>>>>>>>>> net-next since we now trigger a build failure in the kernel
>>>>>>>>>> if
>>>>>>>>>> any
>>>>>>>>>> new
>>>>>>>>>> address families are introduced without adding a
>>>>>>>>>> corresponding
>>>>>>>>>> security
>>>>>>>>>> class (so that SELinux always supports a separate class per
>>>>>>>>>> network
>>>>>>>>>> address family going forward). So there have been no policy
>>>>>>>>>> patches
>>>>>>>>>> submitted yet to define it in refpolicy even AFAIK.
>>>>>>>>>>
>>>>>>>>>> [1] https://github.com/SELinuxProject/selinux/issues/34
>>>>>>>>
>>>>>>>> So i suppose its an unordered class? or do i have to order this
>>>>>>>> one
>>>>>>>> to avoid future issues?
>>>>>>>>
>>>>>>>>>
>>>>>>>>> BTW, I'm not sure what you did to trigger the problem.  When I
>>>>>>>>> tested
>>>>>>>>> the extended socket classes, I added them to my running policy
>>>>>>>>> via
>>>>>>>>> a
>>>>>>>>> CIL module like this:
>>>>>>>>
>>>>>>>> Well i just added this commit (ignore the typo in the name) which
>>>>>>>> adds the new class to the unordered socket list
>>>>>>>>
>>>>>>>> https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659
>>>>>>>> f347
>>>>>>>> 863f09ec9caeceb56
>>>>>>>>
>>>>>>>> then i built rpms using this script:
>>>>>>>>
>>>>>>>> https://github.com/DefenSec/dssp2-standard/blob/master/support/rp
>>>>>>>> m/ds
>>>>>>>> sp2-standard.sh
>>>>>>>>
>>>>>>>> and just rpm -Uvh the new rpm
>>>>>>>>
>>>>>>>>>
>>>>>>>>> (policycap extended_socket_class)
>>>>>>>>>
>>>>>>>>> (classcommon sctp_socket socket)
>>>>>>>>> (class sctp_socket (node_bind))
>>>>>>>>>
>>>>>>>>> <snip>
>>>>>>>>>
>>>>>>>>> (classcommon qipcrtr_socket socket)
>>>>>>>>> (class qipcrtr_socket ())
>>>>>>>>>
>>>>>>>>> (classorder (unordered sctp_socket icmp_socket ax25_socket
>>>>>>>>> ipx_socket
>>>>>>>>> netrom_socket bridge_socket atmpvc_socket x25_socket
>>>>>>>>> rose_socket
>>>>>>>>> decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
>>>>>>>>> llc_socket ib_socket mpls_socket can_socket tipc_socket
>>>>>>>>> bluetooth_socket iucv_socket rxrpc_socket isdn_socket
>>>>>>>>> phonet_socket
>>>>>>>>> ieee802154_socket caif_socket alg_socket nfc_socket
>>>>>>>>> vsock_socket
>>>>>>>>> kcm_socket qipcrtr_socket))
>>>>>>>>>
>>>>>>>>> The classorder statement at the end ensured that they were
>>>>>>>>> appended
>>>>>>>>> to
>>>>>>>>> the end of the class list and therefore did not break anything.
>>>>>>>>>
>>>>>>>
>>>>>>> Looks like you failed to include a classorder statement for it as I
>>>>>>> did
>>>>>>> above.  That's still required to avoid breaking legacy userspace
>>>>>>> object
>>>>>>> managers.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> No I added "scm_socket" (i renamed it later to smc_socket) to my list
>>>>>> or unordered classorder:
>>>>>>
>>>>>> ;
>>>>>> ; Class order of unordered Linux access vectors
>>>>>> ;
>>>>>>
>>>>>> (classorder
>>>>>>     (unordered
>>>>>>         alg_socket
>>>>>>         atmpvc_socket
>>>>>>         atmsvc_socket
>>>>>>         ax25_socket
>>>>>>         bluetooth_socket
>>>>>>         caif_socket
>>>>>>         can_socket
>>>>>>         decnet_socket
>>>>>>         icmp_socket
>>>>>>         ieee802154_socket
>>>>>>         ipx_socket
>>>>>>         irda_socket
>>>>>>         isdn_socket
>>>>>>         iucv_socket
>>>>>>         kcm_socket
>>>>>>         llc_socket
>>>>>>         netrom_socket
>>>>>>         nfc_socket
>>>>>>         phonet_socket
>>>>>>         pppox_socket
>>>>>>         qipcrtr_socket
>>>>>>         rds_socket
>>>>>>         rose_socket
>>>>>>         rxrpc_socket
>>>>>>         scm_socket
>>>>>>         sctp_socket
>>>>>>         tipc_socket
>>>>>>         vsock_socket
>>>>>>         x25_socket
>>>>>>     )
>>>>>> )
>>>>>
>>
>> You are reordering classes here since you didn't put scm_socket at the end
>> of the list. CIL just adds all of the unordered classes in the order that it
>> parses them to the end of the ordered list. So could it be that sctp_socket,
>> tipc_socket, vsock_socket, x25_socket changing caused the problem?
>>
>> Jim
>>
>>>>> Then I'm confused as to why you encountered breakage.  Doesn't CIL
>>>>> ensure that all of those classes are appended to the existing class
>>>>> list?  In which case it won't disturb the dbus class definitions.
>>>>>
>>>
>>> If I would have left out the classorder entry , then a reboot would - i suppose - not have fixed the issue
>>>
>>>>
>>>> CIL appends unordered classes to the existing class list, so I am not sure
>>>> what is going on.
>>>>
>>>> Jim
>
> What was the meaning of "unordered" if order matters just as much as "ordered"?

With ordered lists you will get an error if the ordered lists cannot be merged. 
So two ordered lists with no common members will cause an error to be reported. 
Likewise, if there is an inconsistency, (A B C) and (C B), there will be an 
error. But unordered lists are just added to the end of the ordered lists in the 
order that CIL sees them.

>
> dbus class is unordered, smc_socket is unordered. how can these two unordered classes cause ordering issues?
>

Because the order in the kernel policy changes. Classes are always ordered in 
the kernel. Unordered classes are useful for user space object managers that use 
selinux_check_access() and can support reordering of classes. In the dbus case, 
which apparently does not support that, you will have to be careful about the 
ordering.

Jim

>>>>
>>>>
>>>>
>>>> --
>>>> James Carter <jwcart2@tycho.nsa.gov>
>>>> National Security Agency
>>>
>>>
>>>
>>> _______________________________________________
>>> Selinux mailing list
>>> Selinux@tycho.nsa.gov
>>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>>>
>>
>>
>> --
>> James Carter <jwcart2@tycho.nsa.gov>
>> National Security Agency
>> _______________________________________________
>> Selinux mailing list
>> Selinux@tycho.nsa.gov
>> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>


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

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

* Re: userspace object manager confused
  2017-03-31 14:53                     ` James Carter
@ 2017-03-31 15:01                       ` Dominick Grift
  0 siblings, 0 replies; 14+ messages in thread
From: Dominick Grift @ 2017-03-31 15:01 UTC (permalink / raw)
  To: selinux

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

On Fri, Mar 31, 2017 at 10:53:00AM -0400, James Carter wrote:
> On 03/31/2017 10:39 AM, Dominick Grift wrote:
> > On Fri, Mar 31, 2017 at 10:30:22AM -0400, James Carter wrote:
> > > On 03/31/2017 10:17 AM, Dominick Grift wrote:
> > > > On Fri, Mar 31, 2017 at 10:12:50AM -0400, James Carter wrote:
> > > > > On 03/31/2017 10:10 AM, Stephen Smalley wrote:
> > > > > > On Fri, 2017-03-31 at 15:59 +0200, Dominick Grift wrote:
> > > > > > > On Fri, Mar 31, 2017 at 09:53:26AM -0400, Stephen Smalley wrote:
> > > > > > > > On Fri, 2017-03-31 at 15:36 +0200, Dominick Grift wrote:
> > > > > > > > > On Fri, Mar 31, 2017 at 09:30:56AM -0400, Stephen Smalley wrote:
> > > > > > > > > > On Fri, 2017-03-31 at 09:25 -0400, Stephen Smalley wrote:
> > > > > > > > > > > On Fri, 2017-03-31 at 14:09 +0200, Dominick Grift wrote:
> > > > > > > > > > > > I vaguely recall that we discussed this issue or at least
> > > > > > > > > > > > that
> > > > > > > > > > > > i
> > > > > > > > > > > > mentioned it here but i can't recall the outcome if any:
> > > > > > > > > > > > 
> > > > > > > > > > > > So today on my rawhide system i noticed that i somehow
> > > > > > > > > > > > forgot
> > > > > > > > > > > > to
> > > > > > > > > > > > add
> > > > > > > > > > > > support for the smc_socket class (i suspect that is part of
> > > > > > > > > > > > the
> > > > > > > > > > > > extended socket class patches)
> > > > > > > > > > > > 
> > > > > > > > > > > > I added the class (which i suppose is unordered like the
> > > > > > > > > > > > othe
> > > > > > > > > > > > extended socket classes) but as soon as I loaded up the
> > > > > > > > > > > > policy
> > > > > > > > > > > > with
> > > > > > > > > > > > the new unordered smc_socket class the system became
> > > > > > > > > > > > unusable.
> > > > > > > > > > > > 
> > > > > > > > > > > > This is because the dbus object manager became confused due
> > > > > > > > > > > > to
> > > > > > > > > > > > my
> > > > > > > > > > > > adding a new (unordered) class at runtime, and that the
> > > > > > > > > > > > dbus
> > > > > > > > > > > > class
> > > > > > > > > > > > was no longer working.
> > > > > > > > > > > > 
> > > > > > > > > > > > Modern systems heavily rely on dbus at the heart and so it
> > > > > > > > > > > > is
> > > > > > > > > > > > undesire-able that this happens.
> > > > > > > > > > > > 
> > > > > > > > > > > > A reboot clears this issue up but adding (unordered)
> > > > > > > > > > > > classes at
> > > > > > > > > > > > runtime should not cause these issues i suspect
> > > > > > > > > > > 
> > > > > > > > > > > dbusd doesn't use selinux_check_access() and therefore does
> > > > > > > > > > > not
> > > > > > > > > > > yet
> > > > > > > > > > > support reordering of their classes/permissions at
> > > > > > > > > > > runtime.  The
> > > > > > > > > > > same
> > > > > > > > > > > is true of all userspace object managers created before
> > > > > > > > > > > selinux_check_access() was introduced - anything that
> > > > > > > > > > > directly
> > > > > > > > > > > calls
> > > > > > > > > > > security_compute_av() or avc_has_perm(). dbusd does call
> > > > > > > > > > > selinux_set_mapping() at startup, so it can correctly handle
> > > > > > > > > > > reordering of classes/permissions across restarts, but not
> > > > > > > > > > > while
> > > > > > > > > > > it
> > > > > > > > > > > is
> > > > > > > > > > > running.  Calling selinux_set_mapping() again upon policy
> > > > > > > > > > > reloads
> > > > > > > > > > > (e.g.
> > > > > > > > > > > from policy_reload_callback() if (event ==
> > > > > > > > > > > AVC_CALLBACK_RESET)
> > > > > > > > > > > before
> > > > > > > > > > > returning) may fix this problem, but requires proper
> > > > > > > > > > > locking.  Even
> > > > > > > > > > > better would be to rid the dbusd selinux implementation of
> > > > > > > > > > > threading
> > > > > > > > > > > entirely, see https://bugs.freedesktop.org/show_bug.cgi?id=92
> > > > > > > > > > > 831#
> > > > > > > > > > > c4
> > > > > > > > > 
> > > > > > > > > Thank you, I suppose i should take this to them then.
> > > > > > > > 
> > > > > > > > No, someone who uses SELinux will need to develop a patch and test
> > > > > > > > it
> > > > > > > > for them; dbusd maintainer doesn't use SELinux himself.
> > > > > > > > 
> > > > > > > > Looks like I'm also wrong about being able to call
> > > > > > > > selinux_set_mapping() from an AVC callback, since
> > > > > > > > selinux_set_mapping()
> > > > > > > > itself calls avc_reset() to flush any cache entries before
> > > > > > > > reloading
> > > > > > > > the mapping, so that would produce infinite
> > > > > > > > recursion.  Sigh.  Could
> > > > > > > > change selinux_set_mapping() to not do that in libselinux, but that
> > > > > > > > won't help on existing releases.  Maybe we should just convert it
> > > > > > > > over
> > > > > > > > to using selinux_check_access() and drop all direct usage of the
> > > > > > > > AVC.
> > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > smc_socket was added by the kernel developers as part of the
> > > > > > > > > > > merge
> > > > > > > > > > > with
> > > > > > > > > > > net-next since we now trigger a build failure in the kernel
> > > > > > > > > > > if
> > > > > > > > > > > any
> > > > > > > > > > > new
> > > > > > > > > > > address families are introduced without adding a
> > > > > > > > > > > corresponding
> > > > > > > > > > > security
> > > > > > > > > > > class (so that SELinux always supports a separate class per
> > > > > > > > > > > network
> > > > > > > > > > > address family going forward). So there have been no policy
> > > > > > > > > > > patches
> > > > > > > > > > > submitted yet to define it in refpolicy even AFAIK.
> > > > > > > > > > > 
> > > > > > > > > > > [1] https://github.com/SELinuxProject/selinux/issues/34
> > > > > > > > > 
> > > > > > > > > So i suppose its an unordered class? or do i have to order this
> > > > > > > > > one
> > > > > > > > > to avoid future issues?
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > BTW, I'm not sure what you did to trigger the problem.  When I
> > > > > > > > > > tested
> > > > > > > > > > the extended socket classes, I added them to my running policy
> > > > > > > > > > via
> > > > > > > > > > a
> > > > > > > > > > CIL module like this:
> > > > > > > > > 
> > > > > > > > > Well i just added this commit (ignore the typo in the name) which
> > > > > > > > > adds the new class to the unordered socket list
> > > > > > > > > 
> > > > > > > > > https://github.com/DefenSec/dssp2-base/commit/5e3ada7d12525c8c659
> > > > > > > > > f347
> > > > > > > > > 863f09ec9caeceb56
> > > > > > > > > 
> > > > > > > > > then i built rpms using this script:
> > > > > > > > > 
> > > > > > > > > https://github.com/DefenSec/dssp2-standard/blob/master/support/rp
> > > > > > > > > m/ds
> > > > > > > > > sp2-standard.sh
> > > > > > > > > 
> > > > > > > > > and just rpm -Uvh the new rpm
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > (policycap extended_socket_class)
> > > > > > > > > > 
> > > > > > > > > > (classcommon sctp_socket socket)
> > > > > > > > > > (class sctp_socket (node_bind))
> > > > > > > > > > 
> > > > > > > > > > <snip>
> > > > > > > > > > 
> > > > > > > > > > (classcommon qipcrtr_socket socket)
> > > > > > > > > > (class qipcrtr_socket ())
> > > > > > > > > > 
> > > > > > > > > > (classorder (unordered sctp_socket icmp_socket ax25_socket
> > > > > > > > > > ipx_socket
> > > > > > > > > > netrom_socket bridge_socket atmpvc_socket x25_socket
> > > > > > > > > > rose_socket
> > > > > > > > > > decnet_socket atmsvc_socket rds_socket irda_socket pppox_socket
> > > > > > > > > > llc_socket ib_socket mpls_socket can_socket tipc_socket
> > > > > > > > > > bluetooth_socket iucv_socket rxrpc_socket isdn_socket
> > > > > > > > > > phonet_socket
> > > > > > > > > > ieee802154_socket caif_socket alg_socket nfc_socket
> > > > > > > > > > vsock_socket
> > > > > > > > > > kcm_socket qipcrtr_socket))
> > > > > > > > > > 
> > > > > > > > > > The classorder statement at the end ensured that they were
> > > > > > > > > > appended
> > > > > > > > > > to
> > > > > > > > > > the end of the class list and therefore did not break anything.
> > > > > > > > > > 
> > > > > > > > 
> > > > > > > > Looks like you failed to include a classorder statement for it as I
> > > > > > > > did
> > > > > > > > above.  That's still required to avoid breaking legacy userspace
> > > > > > > > object
> > > > > > > > managers.
> > > > > > > > 
> > > > > > > > 
> > > > > > > 
> > > > > > > No I added "scm_socket" (i renamed it later to smc_socket) to my list
> > > > > > > or unordered classorder:
> > > > > > > 
> > > > > > > ;
> > > > > > > ; Class order of unordered Linux access vectors
> > > > > > > ;
> > > > > > > 
> > > > > > > (classorder
> > > > > > >     (unordered
> > > > > > >         alg_socket
> > > > > > >         atmpvc_socket
> > > > > > >         atmsvc_socket
> > > > > > >         ax25_socket
> > > > > > >         bluetooth_socket
> > > > > > >         caif_socket
> > > > > > >         can_socket
> > > > > > >         decnet_socket
> > > > > > >         icmp_socket
> > > > > > >         ieee802154_socket
> > > > > > >         ipx_socket
> > > > > > >         irda_socket
> > > > > > >         isdn_socket
> > > > > > >         iucv_socket
> > > > > > >         kcm_socket
> > > > > > >         llc_socket
> > > > > > >         netrom_socket
> > > > > > >         nfc_socket
> > > > > > >         phonet_socket
> > > > > > >         pppox_socket
> > > > > > >         qipcrtr_socket
> > > > > > >         rds_socket
> > > > > > >         rose_socket
> > > > > > >         rxrpc_socket
> > > > > > >         scm_socket
> > > > > > >         sctp_socket
> > > > > > >         tipc_socket
> > > > > > >         vsock_socket
> > > > > > >         x25_socket
> > > > > > >     )
> > > > > > > )
> > > > > > 
> > > 
> > > You are reordering classes here since you didn't put scm_socket at the end
> > > of the list. CIL just adds all of the unordered classes in the order that it
> > > parses them to the end of the ordered list. So could it be that sctp_socket,
> > > tipc_socket, vsock_socket, x25_socket changing caused the problem?
> > > 
> > > Jim
> > > 
> > > > > > Then I'm confused as to why you encountered breakage.  Doesn't CIL
> > > > > > ensure that all of those classes are appended to the existing class
> > > > > > list?  In which case it won't disturb the dbus class definitions.
> > > > > > 
> > > > 
> > > > If I would have left out the classorder entry , then a reboot would - i suppose - not have fixed the issue
> > > > 
> > > > > 
> > > > > CIL appends unordered classes to the existing class list, so I am not sure
> > > > > what is going on.
> > > > > 
> > > > > Jim
> > 
> > What was the meaning of "unordered" if order matters just as much as "ordered"?
> 
> With ordered lists you will get an error if the ordered lists cannot be
> merged. So two ordered lists with no common members will cause an error to
> be reported. Likewise, if there is an inconsistency, (A B C) and (C B),
> there will be an error. But unordered lists are just added to the end of the
> ordered lists in the order that CIL sees them.
> 
> > 
> > dbus class is unordered, smc_socket is unordered. how can these two unordered classes cause ordering issues?
> > 
> 
> Because the order in the kernel policy changes. Classes are always ordered
> in the kernel. Unordered classes are useful for user space object managers
> that use selinux_check_access() and can support reordering of classes. In
> the dbus case, which apparently does not support that, you will have to be
> careful about the ordering.

Thanks, so the issue is just object managers that dont use selinux_access_check()

> 
> Jim
> 
> > > > > 
> > > > > 
> > > > > 
> > > > > --
> > > > > James Carter <jwcart2@tycho.nsa.gov>
> > > > > National Security Agency
> > > > 
> > > > 
> > > > 
> > > > _______________________________________________
> > > > Selinux mailing list
> > > > Selinux@tycho.nsa.gov
> > > > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > > > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
> > > > 
> > > 
> > > 
> > > --
> > > James Carter <jwcart2@tycho.nsa.gov>
> > > National Security Agency
> > > _______________________________________________
> > > Selinux mailing list
> > > Selinux@tycho.nsa.gov
> > > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
> > 
> > 
> > 
> > _______________________________________________
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
> > 
> 
> 
> -- 
> James Carter <jwcart2@tycho.nsa.gov>
> National Security Agency

-- 
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] 14+ messages in thread

end of thread, other threads:[~2017-03-31 15:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-31 12:09 userspace object manager confused Dominick Grift
2017-03-31 13:25 ` Stephen Smalley
2017-03-31 13:30   ` Stephen Smalley
2017-03-31 13:36     ` Dominick Grift
2017-03-31 13:53       ` Stephen Smalley
2017-03-31 13:59         ` Dominick Grift
2017-03-31 14:10           ` Stephen Smalley
2017-03-31 14:12             ` James Carter
2017-03-31 14:17               ` Dominick Grift
2017-03-31 14:30                 ` James Carter
2017-03-31 14:39                   ` Dominick Grift
2017-03-31 14:53                     ` James Carter
2017-03-31 15:01                       ` Dominick Grift
2017-03-31 14:32               ` Dominick Grift

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.