"selinux: support distinctions among all network address families" causing existing bluetooth sepolicies to not work properly with Android?

"selinux: support distinctions among all network address families" causing existing bluetooth sepolicies to not work properly with Android?

[John Stultz]

Hey folks,

Recently I was working to validate/enable a new bluetooth HAL on HiKey
with Android, and after getting it working properly with a 4.9 based
kernel, I found that I was seeing failures trying to run with an
upstream (4.12-rc3 based) kernel.

It seemed a call to:
   socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);

was suddenly failing, and running "setenforce 0" would allow it to
continue properly.

I chased the issue down to  da69a5306ab9 ("selinux: support
distinctions among all network address families"). And work around it
with the following (whitespace corrupted, sorry) hack:

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..42dfd0f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1379,8 +1379,8 @@ static inline u16
socket_type_to_security_class(int family, int type, int protoc
                        return SECCLASS_CAN_SOCKET;
                case PF_TIPC:
                        return SECCLASS_TIPC_SOCKET;
-               case PF_BLUETOOTH:
-                       return SECCLASS_BLUETOOTH_SOCKET;
+//             case PF_BLUETOOTH:
+//                     return SECCLASS_BLUETOOTH_SOCKET;
                case PF_IUCV:
                        return SECCLASS_IUCV_SOCKET;
                case PF_RXRPC:

Obviously this isn't ideal. The commit message claims that " Backward
compatibility is provided by only enabling the finer-grained socket
classes if a new policy capability is set in the policy; older
policies will behave as before."

Which makes it seem like the older sepolicy should be fine with newer
kernels, but this doesn't seem to be the case here? Am I missing
something? Is Android doing something odd with their POLICYDB that is
causing the kernel to think the sepolicy is newer?

thanks
-john

Re: "selinux: support distinctions among all network address families" causing existing bluetooth sepolicies to not work properly with Android?

[Stephen Smalley]

On Tue, 2017-06-06 at 17:45 -0700, John Stultz wrote:
> Hey folks,
> 
> Recently I was working to validate/enable a new bluetooth HAL on
> HiKey
> with Android, and after getting it working properly with a 4.9 based
> kernel, I found that I was seeing failures trying to run with an
> upstream (4.12-rc3 based) kernel.
> 
> It seemed a call to:
>    socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
> 
> was suddenly failing, and running "setenforce 0" would allow it to
> continue properly.
> 
> I chased the issue down to  da69a5306ab9 ("selinux: support
> distinctions among all network address families"). And work around it
> with the following (whitespace corrupted, sorry) hack:
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..42dfd0f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1379,8 +1379,8 @@ static inline u16
> socket_type_to_security_class(int family, int type, int protoc
>                         return SECCLASS_CAN_SOCKET;
>                 case PF_TIPC:
>                         return SECCLASS_TIPC_SOCKET;
> -               case PF_BLUETOOTH:
> -                       return SECCLASS_BLUETOOTH_SOCKET;
> +//             case PF_BLUETOOTH:
> +//                     return SECCLASS_BLUETOOTH_SOCKET;
>                 case PF_IUCV:
>                         return SECCLASS_IUCV_SOCKET;
>                 case PF_RXRPC:
> 
> Obviously this isn't ideal. The commit message claims that " Backward
> compatibility is provided by only enabling the finer-grained socket
> classes if a new policy capability is set in the policy; older
> policies will behave as before."
> 
> Which makes it seem like the older sepolicy should be fine with newer
> kernels, but this doesn't seem to be the case here? Am I missing
> something? Is Android doing something odd with their POLICYDB that is
> causing the kernel to think the sepolicy is newer?

The code above is only enabled if the policy enables the
extended_socket_class policy capability.  I added that to AOSP policy
in 431bdd9f2f344ecde4cd3fe0109bd70eab0a394c.  The correct fix is not to
change the kernel but rather to add allow rules to policy for the
finer-grained socket classes.  Your dmesg or logcat output will show
you the denials, and audit2allow can help with an initial cut at rules,
although you should refine them to use the macros and, where
appropriate, attributes.

Re: "selinux: support distinctions among all network address families" causing existing bluetooth sepolicies to not work properly with Android?

[Stephen Smalley]

On Wed, 2017-06-07 at 08:40 -0400, Stephen Smalley wrote:
> On Tue, 2017-06-06 at 17:45 -0700, John Stultz wrote:
> > Hey folks,
> > 
> > Recently I was working to validate/enable a new bluetooth HAL on
> > HiKey
> > with Android, and after getting it working properly with a 4.9
> > based
> > kernel, I found that I was seeing failures trying to run with an
> > upstream (4.12-rc3 based) kernel.
> > 
> > It seemed a call to:
> >    socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
> > 
> > was suddenly failing, and running "setenforce 0" would allow it to
> > continue properly.
> > 
> > I chased the issue down to  da69a5306ab9 ("selinux: support
> > distinctions among all network address families"). And work around
> > it
> > with the following (whitespace corrupted, sorry) hack:
> > 
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526..42dfd0f 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -1379,8 +1379,8 @@ static inline u16
> > socket_type_to_security_class(int family, int type, int protoc
> >                         return SECCLASS_CAN_SOCKET;
> >                 case PF_TIPC:
> >                         return SECCLASS_TIPC_SOCKET;
> > -               case PF_BLUETOOTH:
> > -                       return SECCLASS_BLUETOOTH_SOCKET;
> > +//             case PF_BLUETOOTH:
> > +//                     return SECCLASS_BLUETOOTH_SOCKET;
> >                 case PF_IUCV:
> >                         return SECCLASS_IUCV_SOCKET;
> >                 case PF_RXRPC:
> > 
> > Obviously this isn't ideal. The commit message claims that "
> > Backward
> > compatibility is provided by only enabling the finer-grained socket
> > classes if a new policy capability is set in the policy; older
> > policies will behave as before."
> > 
> > Which makes it seem like the older sepolicy should be fine with
> > newer
> > kernels, but this doesn't seem to be the case here? Am I missing
> > something? Is Android doing something odd with their POLICYDB that
> > is
> > causing the kernel to think the sepolicy is newer?
> 
> The code above is only enabled if the policy enables the
> extended_socket_class policy capability.  I added that to AOSP policy
> in 431bdd9f2f344ecde4cd3fe0109bd70eab0a394c.  The correct fix is not
> to
> change the kernel but rather to add allow rules to policy for the
> finer-grained socket classes.  Your dmesg or logcat output will show
> you the denials, and audit2allow can help with an initial cut at
> rules,
> although you should refine them to use the macros and, where
> appropriate, attributes.

To elaborate a bit further: the backward compatibility provision is for
policies that predate the introduction of the extended_socket_class
policy capability or that do not opt into it.  Thus, for example,
Android N and earlier had policies that did not define this capability
and therefore will fall back to the old behavior and remain compatible.
 Android O appears to have included the aforementioned sepolicy commit
from AOSP master and therefore has opted into this functionality and
must provide appropriate allow rules for it when using a kernel that
includes this functionality. To do so, one needs to review all allow
rules on 'socket' and duplicate them with allow rules on the more
specific per-address-family security classes that are required (which
can be determined either through testing or code examination). 
Eventually, the original rules on the generic 'socket' class can be
removed from sepolicy entirely, but not until support for older kernels
that predate this capability is no longer needed.  One also needs to
review all allow rules on rawip_socket and determine whether they
should be duplicated for sctp_socket and/or icmp_socket if the program
was in fact using a SCTP or an unprivileged ICMP (ping) socket.

Re: "selinux: support distinctions among all network address families" causing existing bluetooth sepolicies to not work properly with Android?

[John Stultz]

On Wed, Jun 7, 2017 at 7:44 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2017-06-07 at 08:40 -0400, Stephen Smalley wrote:
>> On Tue, 2017-06-06 at 17:45 -0700, John Stultz wrote:
>> > Hey folks,
>> >
>> > Recently I was working to validate/enable a new bluetooth HAL on
>> > HiKey
>> > with Android, and after getting it working properly with a 4.9
>> > based
>> > kernel, I found that I was seeing failures trying to run with an
>> > upstream (4.12-rc3 based) kernel.
>> >
>> > It seemed a call to:
>> >    socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
>> >
>> > was suddenly failing, and running "setenforce 0" would allow it to
>> > continue properly.
>> >
>> > I chased the issue down to  da69a5306ab9 ("selinux: support
>> > distinctions among all network address families"). And work around
>> > it
>> > with the following (whitespace corrupted, sorry) hack:
>> >
>> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> > index e67a526..42dfd0f 100644
>> > --- a/security/selinux/hooks.c
>> > +++ b/security/selinux/hooks.c
>> > @@ -1379,8 +1379,8 @@ static inline u16
>> > socket_type_to_security_class(int family, int type, int protoc
>> >                         return SECCLASS_CAN_SOCKET;
>> >                 case PF_TIPC:
>> >                         return SECCLASS_TIPC_SOCKET;
>> > -               case PF_BLUETOOTH:
>> > -                       return SECCLASS_BLUETOOTH_SOCKET;
>> > +//             case PF_BLUETOOTH:
>> > +//                     return SECCLASS_BLUETOOTH_SOCKET;
>> >                 case PF_IUCV:
>> >                         return SECCLASS_IUCV_SOCKET;
>> >                 case PF_RXRPC:
>> >
>> > Obviously this isn't ideal. The commit message claims that "
>> > Backward
>> > compatibility is provided by only enabling the finer-grained socket
>> > classes if a new policy capability is set in the policy; older
>> > policies will behave as before."
>> >
>> > Which makes it seem like the older sepolicy should be fine with
>> > newer
>> > kernels, but this doesn't seem to be the case here? Am I missing
>> > something? Is Android doing something odd with their POLICYDB that
>> > is
>> > causing the kernel to think the sepolicy is newer?
>>
>> The code above is only enabled if the policy enables the
>> extended_socket_class policy capability.  I added that to AOSP policy
>> in 431bdd9f2f344ecde4cd3fe0109bd70eab0a394c.  The correct fix is not
>> to
>> change the kernel but rather to add allow rules to policy for the
>> finer-grained socket classes.  Your dmesg or logcat output will show
>> you the denials, and audit2allow can help with an initial cut at
>> rules,
>> although you should refine them to use the macros and, where
>> appropriate, attributes.
>
> To elaborate a bit further: the backward compatibility provision is for
> policies that predate the introduction of the extended_socket_class
> policy capability or that do not opt into it.  Thus, for example,
> Android N and earlier had policies that did not define this capability
> and therefore will fall back to the old behavior and remain compatible.
>  Android O appears to have included the aforementioned sepolicy commit
> from AOSP master and therefore has opted into this functionality and
> must provide appropriate allow rules for it when using a kernel that
> includes this functionality. To do so, one needs to review all allow
> rules on 'socket' and duplicate them with allow rules on the more
> specific per-address-family security classes that are required (which
> can be determined either through testing or code examination).
> Eventually, the original rules on the generic 'socket' class can be
> removed from sepolicy entirely, but not until support for older kernels
> that predate this capability is no longer needed.  One also needs to
> review all allow rules on rawip_socket and determine whether they
> should be duplicated for sctp_socket and/or icmp_socket if the program
> was in fact using a SCTP or an unprivileged ICMP (ping) socket.


Ah. Ok. Very much appreciate the extra details here! I wasn't
expecting Android's userspace to already have enabled the new sepolicy
rules, so I assumed it must have been some other misstep.

I've reworked the problematic policy file and its now working for me!

Thanks again!
-john