Received-SPF: pass (google.com: domain of selinux-refpolicy+bounces-11-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <4140be53-76bc-4aee-880f-4799b996897b@ieee.org>
Date: Wed, 15 May 2024 09:51:14 -0400
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1] Adding Sepolicy rules to allow bluetoothctl and
 dbus-daemon to access unix stream sockets.
To: Naga Bhavani Akella <quic_nakella@quicinc.com>,
 selinux-refpolicy@vger.kernel.org
Cc: quic_mohamull@quicinc.com, quic_hbandi@quicinc.com,
 quic_anubhavg@quicinc.com
References: <20240514072123.11376-1-quic_nakella@quicinc.com>
 <877cf03e-037d-4708-8ac0-bce5b2caa48a@ieee.org>
 <695f447e-b9db-414f-8fa5-1b12b8379c48@quicinc.com>
Content-Language: en-US
From: Chris PeBenito <pebenito@ieee.org>
In-Reply-To: <695f447e-b9db-414f-8fa5-1b12b8379c48@quicinc.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 5/15/2024 3:29 AM, Naga Bhavani Akella wrote:
> Hi Chris PeBenito,
> 
> Thank you for your comments.
> 
> On 5/15/2024 1:42 AM, Chris PeBenito wrote:
>> On 5/14/2024 3:21 AM, Naga Bhavani Akella wrote:
>>> Required for using acquire-notify, acquire-write options (Gatt Client)
>>> and Sending notifications (Gatt Server)
>>>
>>> Below are the avc denials that are fixed with this patch -
>>>
>>> 1. audit: type=1400 audit(1651238006.276:496):
>>> avc:  denied  { read write } for  pid=2165 comm="bluetoothd"
>>> path="socket:[43207]" dev="sockfs" ino=43207
>>> scontext=system_u:system_r:bluetooth_t:s0-s15:c0.c1023
>>> tcontext=system_u:system_r:initrc_t:s0-s15:c0.c1023
>>> tclass=unix_stream_socket permissive=1
>>> 2. audit: type=1400 audit(1651238006.276:497):
>>> avc:  denied  { getattr } for  pid=2165 comm="bluetoothd"
>>> path="socket:[43207]" dev="sockfs" ino=43207
>>> scontext=system_u:system_r:bluetooth_t:s0-s15:c0.c1023
>>> tcontext=system_u:system_r:initrc_t:s0-s15:c0.c1023
>>> tclass=unix_stream_socket permissive=1
>>> 3. audit: type=1400 audit(1651238006.272:495):
>>> avc:  denied  { read write } for  pid=689 comm="dbus-daemon"
>>> path="socket:[43207]" dev="sockfs" ino=43207
>>> scontext=system_u:system_r:system_dbusd_t:s0-s15:c0.c1023
>>> tcontext=system_u:system_r:initrc_t:s0-s15:c0.c1023
>>> tclass=unix_stream_socket permissive=1
>>> 4. audit: type=1400 audit(315966559.395:444):
>>> avc:  denied  { use } for  pid=710 comm="dbus-daemon"
>>> path="socket:[13196]" dev="sockfs" ino=13196
>>> scontext=system_u:system_r:system_dbusd_t:s0-s15:c0.c1023
>>> tcontext=system_u:system_r:bluetooth_t:s0-s15:c0.c1023
>>> tclass=fd permissive=0
>>> 5. audit: type=1400 audit(315999854.939:523):
>>> avc:  denied  { read write } for  pid=812 comm="dbus-daemon"
>>> path="socket:[99469]" dev="sockfs" ino=99469
>>> scontext=system_u:system_r:system_dbusd_t:s0-s15:c0.c1023
>>> tcontext=system_u:system_r:bluetooth_t:s0-s15:c0.c1023
>>> tclass=bluetooth_socket permissive=1
>>>
>>> Signed-off-by: Naga Bhavani Akella <quic_nakella@quicinc.com>
>>> ---
>>>    policy/modules/services/bluetooth.fc | 1 +
>>>    policy/modules/services/bluetooth.if | 3 +++
>>>    2 files changed, 4 insertions(+)
>>>
>>> diff --git a/policy/modules/services/bluetooth.fc b/policy/modules/services/bluetooth.fc
>>> index e167e93f7..03f42fc8b 100644
>>> --- a/policy/modules/services/bluetooth.fc
>>> +++ b/policy/modules/services/bluetooth.fc
>>> @@ -7,6 +7,7 @@
>>>      /usr/bin/blue.*pin    --    gen_context(system_u:object_r:bluetooth_helper_exec_t,s0)
>>>    /usr/bin/bluetoothd    --    gen_context(system_u:object_r:bluetooth_exec_t,s0)
>>> +/usr/bin/bluetoothctl    --    gen_context(system_u:object_r:bluetooth_exec_t,s0)
>>>    /usr/bin/dund    --    gen_context(system_u:object_r:bluetooth_exec_t,s0)
>>>    /usr/bin/hciattach    --    gen_context(system_u:object_r:bluetooth_exec_t,s0)
>>>    /usr/bin/hcid    --    gen_context(system_u:object_r:bluetooth_exec_t,s0)
>>> diff --git a/policy/modules/services/bluetooth.if b/policy/modules/services/bluetooth.if
>>> index c7e1c3f14..c869eed71 100644
>>> --- a/policy/modules/services/bluetooth.if
>>> +++ b/policy/modules/services/bluetooth.if
>>> @@ -82,6 +82,9 @@ interface(`bluetooth_stream_connect',`
>>>          files_search_runtime($1)
>>>        allow $1 bluetooth_t:socket rw_socket_perms;
>>> +    allow $1 bluetooth_t:bluetooth_socket rw_socket_perms;
>>> +    allow $1 bluetooth_t:unix_stream_socket { accept connectto listen rw_socket_perms };
>>> +    allow $1 bluetooth_t:fd use;
>>>        stream_connect_pattern($1, bluetooth_runtime_t, bluetooth_runtime_t, bluetooth_t)
>>>    ')
>>
>> I have two main concerns:
>>
>> 1. is the added access in this interface required for all users that connect to bluetoothd over a unix domain socket?
>>
> As per my understanding any user who wants to use bluetooth over unix domain has to make dbus calls to bluetoothd. So I feel its required. For example- bluetooth_stream_connect is currently being used by pulseaudio_t and obex_t as well, these services also use dbus methods.
> Please let us know your opinion on this or better way to fix these AVC denials.

In that case, then a new interface with a more abstract name would be 
warranted.


>> 2. does bluetoothctl really need all the access of the bluetooth_t domain?  It may only need a small permission set, like udev and udevctl.
>>
> By default bluetoothctl is running in initrc_t domain so added bluetooth_exec_t domain to bluetoothctl. bluetoothctl is command line tool to access whole bluetooth functionality like pair/unpair/connect/disconnect etc. There are other tools like hciattach, rfcomm which are using same domain bluetooth_exec_t.

Yes, the point is that we probably need a bluetoothctl_t domain so the 
configuration can be done only via the bluetoothctl process, not just 
any initrc_t process.  The existing bluetooth_helper_t domain may 
possibly be renamed/retrofitted for this purpose.


-- 
Chris PeBenito