[PATCH] Bluetooth: hci_sync: Set Privacy Mode when updating the resolving list

[PATCH] Bluetooth: hci_sync: Set Privacy Mode when updating the resolving list

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds support for Set Privacy Mode when updating the resolving list
when HCI_LIMITED_PRIVACY so the controller shall use Device Mode for
devices programmed in the resolving list, Device Mode is actually
required when the remote device are not able to use RPA as otherwise the
default mode is Network Privacy Mode in which only RPA are allowed thus
the controller would filter out advertisement using the identity address
for which there is an IRK.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/hci.h |  7 ++++++
 net/bluetooth/hci_sync.c    | 48 ++++++++++++++++++++++++++++++++-----
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b76..aa856dfd5b9f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1930,6 +1930,13 @@ struct hci_rp_le_read_transmit_power {
 	__s8  max_le_tx_power;
 } __packed;
 
+#define HCI_OP_LE_SET_PRIVACY_MODE	0x204e
+struct hci_cp_le_set_privacy_mode {
+	__u8  bdaddr_type;
+	bdaddr_t  bdaddr;
+	__u8  mode;
+} __packed;
+
 #define HCI_OP_LE_READ_BUFFER_SIZE_V2	0x2060
 struct hci_rp_le_read_buffer_size_v2 {
 	__u8    status;
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index b794605dc882..43173d645436 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -1580,8 +1580,37 @@ static int hci_le_add_resolve_list_sync(struct hci_dev *hdev,
 				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
 }
 
+/* Set Device Privacy Mode. */
+static int hci_le_set_privacy_mode_sync(struct hci_dev *hdev,
+					struct hci_conn_params *params)
+{
+	struct hci_cp_le_set_privacy_mode cp;
+	struct smp_irk *irk;
+
+	/* Set Privacy Mode requires the use of resolving list (aka. LL Privacy)
+	 * by default Network Mode is used so only really send the command if
+	 * Device Mode is required (HCI_LIMITED_PRIVACY).
+	 */
+	if (!use_ll_privacy(hdev) ||
+	    !hci_dev_test_flag(hdev, HCI_LIMITED_PRIVACY))
+		return 0;
+
+	irk = hci_find_irk_by_addr(hdev, &params->addr, params->addr_type);
+	if (!irk)
+		return 0;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.bdaddr_type = irk->addr_type;
+	bacpy(&cp.bdaddr, &irk->bdaddr);
+	cp.mode = 0x01;
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_PRIVACY_MODE,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+}
+
 /* Adds connection to allow list if needed, if the device uses RPA (has IRK)
- * this attempts to program the device in the resolving list as well.
+ * this attempts to program the device in the resolving list as well and
+ * properly set the privacy mode.
  */
 static int hci_le_add_accept_list_sync(struct hci_dev *hdev,
 				       struct hci_conn_params *params,
@@ -1590,11 +1619,6 @@ static int hci_le_add_accept_list_sync(struct hci_dev *hdev,
 	struct hci_cp_le_add_to_accept_list cp;
 	int err;
 
-	/* Already in accept list */
-	if (hci_bdaddr_list_lookup(&hdev->le_accept_list, &params->addr,
-				   params->addr_type))
-		return 0;
-
 	/* Select filter policy to accept all advertising */
 	if (*num_entries >= hdev->le_accept_list_size)
 		return -ENOSPC;
@@ -1620,6 +1644,18 @@ static int hci_le_add_accept_list_sync(struct hci_dev *hdev,
 		return err;
 	}
 
+	/* Set Privacy Mode */
+	err = hci_le_set_privacy_mode_sync(hdev, params);
+	if (err) {
+		bt_dev_err(hdev, "Unable to set privacy mode: %d", err);
+		return err;
+	}
+
+	/* Check if already in accept list */
+	if (hci_bdaddr_list_lookup(&hdev->le_accept_list, &params->addr,
+				   params->addr_type))
+		return 0;
+
 	*num_entries += 1;
 	cp.bdaddr_type = params->addr_type;
 	bacpy(&cp.bdaddr, &params->addr);
-- 
2.31.1

Re: [PATCH] Bluetooth: hci_sync: Set Privacy Mode when updating the resolving list

[Marcel Holtmann]

Hi Luiz,

> This adds support for Set Privacy Mode when updating the resolving list
> when HCI_LIMITED_PRIVACY so the controller shall use Device Mode for
> devices programmed in the resolving list, Device Mode is actually
> required when the remote device are not able to use RPA as otherwise the
> default mode is Network Privacy Mode in which only RPA are allowed thus
> the controller would filter out advertisement using the identity address
> for which there is an IRK.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/hci.h |  7 ++++++
> net/bluetooth/hci_sync.c    | 48 ++++++++++++++++++++++++++++++++-----
> 2 files changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 63065bc01b76..aa856dfd5b9f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1930,6 +1930,13 @@ struct hci_rp_le_read_transmit_power {
> 	__s8  max_le_tx_power;
> } __packed;
> 
> +#define HCI_OP_LE_SET_PRIVACY_MODE	0x204e
> +struct hci_cp_le_set_privacy_mode {
> +	__u8  bdaddr_type;
> +	bdaddr_t  bdaddr;
> +	__u8  mode;
> +} __packed;
> +
> #define HCI_OP_LE_READ_BUFFER_SIZE_V2	0x2060
> struct hci_rp_le_read_buffer_size_v2 {
> 	__u8    status;
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index b794605dc882..43173d645436 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -1580,8 +1580,37 @@ static int hci_le_add_resolve_list_sync(struct hci_dev *hdev,
> 				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> }
> 
> +/* Set Device Privacy Mode. */
> +static int hci_le_set_privacy_mode_sync(struct hci_dev *hdev,
> +					struct hci_conn_params *params)
> +{
> +	struct hci_cp_le_set_privacy_mode cp;
> +	struct smp_irk *irk;
> +
> +	/* Set Privacy Mode requires the use of resolving list (aka. LL Privacy)
> +	 * by default Network Mode is used so only really send the command if
> +	 * Device Mode is required (HCI_LIMITED_PRIVACY).
> +	 */
> +	if (!use_ll_privacy(hdev) ||
> +	    !hci_dev_test_flag(hdev, HCI_LIMITED_PRIVACY))
> +		return 0;
> +
> +	irk = hci_find_irk_by_addr(hdev, &params->addr, params->addr_type);
> +	if (!irk)
> +		return 0;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	cp.bdaddr_type = irk->addr_type;
> +	bacpy(&cp.bdaddr, &irk->bdaddr);
> +	cp.mode = 0x01;
> +

you need to check if this command is actually supported.

I think the best option is to add it to Set Device Flags and let bluetoothd set it on a per device basis like it does with the wakeup flag.

Trying to tie it to the Limited Privacy mode seems weird. Since that is for discoverability and this is for outgoing connections.

Regards

Marcel

Re: [PATCH] Bluetooth: hci_sync: Set Privacy Mode when updating the resolving list

[Luiz Augusto von Dentz]

Hi Marcel,

On Tue, Nov 2, 2021 at 12:18 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > This adds support for Set Privacy Mode when updating the resolving list
> > when HCI_LIMITED_PRIVACY so the controller shall use Device Mode for
> > devices programmed in the resolving list, Device Mode is actually
> > required when the remote device are not able to use RPA as otherwise the
> > default mode is Network Privacy Mode in which only RPA are allowed thus
> > the controller would filter out advertisement using the identity address
> > for which there is an IRK.
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > include/net/bluetooth/hci.h |  7 ++++++
> > net/bluetooth/hci_sync.c    | 48 ++++++++++++++++++++++++++++++++-----
> > 2 files changed, 49 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 63065bc01b76..aa856dfd5b9f 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -1930,6 +1930,13 @@ struct hci_rp_le_read_transmit_power {
> >       __s8  max_le_tx_power;
> > } __packed;
> >
> > +#define HCI_OP_LE_SET_PRIVACY_MODE   0x204e
> > +struct hci_cp_le_set_privacy_mode {
> > +     __u8  bdaddr_type;
> > +     bdaddr_t  bdaddr;
> > +     __u8  mode;
> > +} __packed;
> > +
> > #define HCI_OP_LE_READ_BUFFER_SIZE_V2 0x2060
> > struct hci_rp_le_read_buffer_size_v2 {
> >       __u8    status;
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index b794605dc882..43173d645436 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -1580,8 +1580,37 @@ static int hci_le_add_resolve_list_sync(struct hci_dev *hdev,
> >                                    sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> > }
> >
> > +/* Set Device Privacy Mode. */
> > +static int hci_le_set_privacy_mode_sync(struct hci_dev *hdev,
> > +                                     struct hci_conn_params *params)
> > +{
> > +     struct hci_cp_le_set_privacy_mode cp;
> > +     struct smp_irk *irk;
> > +
> > +     /* Set Privacy Mode requires the use of resolving list (aka. LL Privacy)
> > +      * by default Network Mode is used so only really send the command if
> > +      * Device Mode is required (HCI_LIMITED_PRIVACY).
> > +      */
> > +     if (!use_ll_privacy(hdev) ||
> > +         !hci_dev_test_flag(hdev, HCI_LIMITED_PRIVACY))
> > +             return 0;
> > +
> > +     irk = hci_find_irk_by_addr(hdev, &params->addr, params->addr_type);
> > +     if (!irk)
> > +             return 0;
> > +
> > +     memset(&cp, 0, sizeof(cp));
> > +     cp.bdaddr_type = irk->addr_type;
> > +     bacpy(&cp.bdaddr, &irk->bdaddr);
> > +     cp.mode = 0x01;
> > +
>
> you need to check if this command is actually supported.

The spec suggests it is mandatory if LL Privacy is supported:

'C9: Mandatory if LE Feature (LL Privacy) is supported, otherwise excluded.'

> I think the best option is to add it to Set Device Flags and let bluetoothd set it on a per device basis like it does with the wakeup flag.

Got it, I guess

> Trying to tie it to the Limited Privacy mode seems weird. Since that is for discoverability and this is for outgoing connections.

Hmm, I'm afraid this wouldn't follow the spec to the letter then since
it doesn't actually split Incoming/advertising and Outgoing/scanning
privacy, it does seems to be global so if we go with different
settings to control each direction separately we may end up with a
custom mode e.g: limited discoverability for advertising vs network
privacy mode for scanning, for instance limited discoverability does
blast the public address while discoverable and bondable but both
privacy modes seems to require the use of RPA, anyway it would
probably be a good idea to document the interactions, also main.conf
seem to have gone with privacy from the specification so we may either
need add more modes:

Privacy:
"on"/"network": Apply Network Privacy Mode both for advertising and scanning
"limited-network": Apply Limited Discoverable Mode to advertising and
Network Privacy Mode for scanning
"device": Apply Device Privacy Mode both for advertising and scanning
"limited-device": Apply Limited Discoverable Mode to advertising and
Device Privacy Mode for scanning

-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: hci_sync: Set Privacy Mode when updating the resolving list

[Marcel Holtmann]

Hi Luiz,

>>> This adds support for Set Privacy Mode when updating the resolving list
>>> when HCI_LIMITED_PRIVACY so the controller shall use Device Mode for
>>> devices programmed in the resolving list, Device Mode is actually
>>> required when the remote device are not able to use RPA as otherwise the
>>> default mode is Network Privacy Mode in which only RPA are allowed thus
>>> the controller would filter out advertisement using the identity address
>>> for which there is an IRK.
>>> 
>>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>> ---
>>> include/net/bluetooth/hci.h |  7 ++++++
>>> net/bluetooth/hci_sync.c    | 48 ++++++++++++++++++++++++++++++++-----
>>> 2 files changed, 49 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>> index 63065bc01b76..aa856dfd5b9f 100644
>>> --- a/include/net/bluetooth/hci.h
>>> +++ b/include/net/bluetooth/hci.h
>>> @@ -1930,6 +1930,13 @@ struct hci_rp_le_read_transmit_power {
>>>      __s8  max_le_tx_power;
>>> } __packed;
>>> 
>>> +#define HCI_OP_LE_SET_PRIVACY_MODE   0x204e
>>> +struct hci_cp_le_set_privacy_mode {
>>> +     __u8  bdaddr_type;
>>> +     bdaddr_t  bdaddr;
>>> +     __u8  mode;
>>> +} __packed;
>>> +
>>> #define HCI_OP_LE_READ_BUFFER_SIZE_V2 0x2060
>>> struct hci_rp_le_read_buffer_size_v2 {
>>>      __u8    status;
>>> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
>>> index b794605dc882..43173d645436 100644
>>> --- a/net/bluetooth/hci_sync.c
>>> +++ b/net/bluetooth/hci_sync.c
>>> @@ -1580,8 +1580,37 @@ static int hci_le_add_resolve_list_sync(struct hci_dev *hdev,
>>>                                   sizeof(cp), &cp, HCI_CMD_TIMEOUT);
>>> }
>>> 
>>> +/* Set Device Privacy Mode. */
>>> +static int hci_le_set_privacy_mode_sync(struct hci_dev *hdev,
>>> +                                     struct hci_conn_params *params)
>>> +{
>>> +     struct hci_cp_le_set_privacy_mode cp;
>>> +     struct smp_irk *irk;
>>> +
>>> +     /* Set Privacy Mode requires the use of resolving list (aka. LL Privacy)
>>> +      * by default Network Mode is used so only really send the command if
>>> +      * Device Mode is required (HCI_LIMITED_PRIVACY).
>>> +      */
>>> +     if (!use_ll_privacy(hdev) ||
>>> +         !hci_dev_test_flag(hdev, HCI_LIMITED_PRIVACY))
>>> +             return 0;
>>> +
>>> +     irk = hci_find_irk_by_addr(hdev, &params->addr, params->addr_type);
>>> +     if (!irk)
>>> +             return 0;
>>> +
>>> +     memset(&cp, 0, sizeof(cp));
>>> +     cp.bdaddr_type = irk->addr_type;
>>> +     bacpy(&cp.bdaddr, &irk->bdaddr);
>>> +     cp.mode = 0x01;
>>> +
>> 
>> you need to check if this command is actually supported.
> 
> The spec suggests it is mandatory if LL Privacy is supported:
> 
> 'C9: Mandatory if LE Feature (LL Privacy) is supported, otherwise excluded.'

and what about Bluetooth 4.2 spec. devices? Not all earlier specs have both features.

>> I think the best option is to add it to Set Device Flags and let bluetoothd set it on a per device basis like it does with the wakeup flag.
> 
> Got it, I guess

I prefer Set Device Flags since with Get Device Flags you can indicate if this is supported or not.

>> Trying to tie it to the Limited Privacy mode seems weird. Since that is for discoverability and this is for outgoing connections.
> 
> Hmm, I'm afraid this wouldn't follow the spec to the letter then since
> it doesn't actually split Incoming/advertising and Outgoing/scanning
> privacy, it does seems to be global so if we go with different
> settings to control each direction separately we may end up with a
> custom mode e.g: limited discoverability for advertising vs network
> privacy mode for scanning, for instance limited discoverability does
> blast the public address while discoverable and bondable but both
> privacy modes seems to require the use of RPA, anyway it would
> probably be a good idea to document the interactions, also main.conf
> seem to have gone with privacy from the specification so we may either
> need add more modes:
> 
> Privacy:
> "on"/"network": Apply Network Privacy Mode both for advertising and scanning
> "limited-network": Apply Limited Discoverable Mode to advertising and
> Network Privacy Mode for scanning
> "device": Apply Device Privacy Mode both for advertising and scanning
> "limited-device": Apply Limited Discoverable Mode to advertising and
> Device Privacy Mode for scanning

Then we can do that and use Limited Privacy mode for exactly that. However in addition I would use the Set Device Flags to allow punching holes for specific devices.

Regards

Marcel

Re: [PATCH] Bluetooth: hci_sync: Set Privacy Mode when updating the resolving list

[Luiz Augusto von Dentz]

Hi Marcel,

On Wed, Nov 3, 2021 at 1:28 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> >>> This adds support for Set Privacy Mode when updating the resolving list
> >>> when HCI_LIMITED_PRIVACY so the controller shall use Device Mode for
> >>> devices programmed in the resolving list, Device Mode is actually
> >>> required when the remote device are not able to use RPA as otherwise the
> >>> default mode is Network Privacy Mode in which only RPA are allowed thus
> >>> the controller would filter out advertisement using the identity address
> >>> for which there is an IRK.
> >>>
> >>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>> ---
> >>> include/net/bluetooth/hci.h |  7 ++++++
> >>> net/bluetooth/hci_sync.c    | 48 ++++++++++++++++++++++++++++++++-----
> >>> 2 files changed, 49 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >>> index 63065bc01b76..aa856dfd5b9f 100644
> >>> --- a/include/net/bluetooth/hci.h
> >>> +++ b/include/net/bluetooth/hci.h
> >>> @@ -1930,6 +1930,13 @@ struct hci_rp_le_read_transmit_power {
> >>>      __s8  max_le_tx_power;
> >>> } __packed;
> >>>
> >>> +#define HCI_OP_LE_SET_PRIVACY_MODE   0x204e
> >>> +struct hci_cp_le_set_privacy_mode {
> >>> +     __u8  bdaddr_type;
> >>> +     bdaddr_t  bdaddr;
> >>> +     __u8  mode;
> >>> +} __packed;
> >>> +
> >>> #define HCI_OP_LE_READ_BUFFER_SIZE_V2 0x2060
> >>> struct hci_rp_le_read_buffer_size_v2 {
> >>>      __u8    status;
> >>> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> >>> index b794605dc882..43173d645436 100644
> >>> --- a/net/bluetooth/hci_sync.c
> >>> +++ b/net/bluetooth/hci_sync.c
> >>> @@ -1580,8 +1580,37 @@ static int hci_le_add_resolve_list_sync(struct hci_dev *hdev,
> >>>                                   sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> >>> }
> >>>
> >>> +/* Set Device Privacy Mode. */
> >>> +static int hci_le_set_privacy_mode_sync(struct hci_dev *hdev,
> >>> +                                     struct hci_conn_params *params)
> >>> +{
> >>> +     struct hci_cp_le_set_privacy_mode cp;
> >>> +     struct smp_irk *irk;
> >>> +
> >>> +     /* Set Privacy Mode requires the use of resolving list (aka. LL Privacy)
> >>> +      * by default Network Mode is used so only really send the command if
> >>> +      * Device Mode is required (HCI_LIMITED_PRIVACY).
> >>> +      */
> >>> +     if (!use_ll_privacy(hdev) ||
> >>> +         !hci_dev_test_flag(hdev, HCI_LIMITED_PRIVACY))
> >>> +             return 0;
> >>> +
> >>> +     irk = hci_find_irk_by_addr(hdev, &params->addr, params->addr_type);
> >>> +     if (!irk)
> >>> +             return 0;
> >>> +
> >>> +     memset(&cp, 0, sizeof(cp));
> >>> +     cp.bdaddr_type = irk->addr_type;
> >>> +     bacpy(&cp.bdaddr, &irk->bdaddr);
> >>> +     cp.mode = 0x01;
> >>> +
> >>
> >> you need to check if this command is actually supported.
> >
> > The spec suggests it is mandatory if LL Privacy is supported:
> >
> > 'C9: Mandatory if LE Feature (LL Privacy) is supported, otherwise excluded.'
>
> and what about Bluetooth 4.2 spec. devices? Not all earlier specs have both features.

Right looks like Privacy Mode was introduced in 5.0 but LL Privacy was
already part of 4.2, so I guess we will need to check if the command
is really supported after all.

> >> I think the best option is to add it to Set Device Flags and let bluetoothd set it on a per device basis like it does with the wakeup flag.
> >
> > Got it, I guess
>
> I prefer Set Device Flags since with Get Device Flags you can indicate if this is supported or not.
>
> >> Trying to tie it to the Limited Privacy mode seems weird. Since that is for discoverability and this is for outgoing connections.
> >
> > Hmm, I'm afraid this wouldn't follow the spec to the letter then since
> > it doesn't actually split Incoming/advertising and Outgoing/scanning
> > privacy, it does seems to be global so if we go with different
> > settings to control each direction separately we may end up with a
> > custom mode e.g: limited discoverability for advertising vs network
> > privacy mode for scanning, for instance limited discoverability does
> > blast the public address while discoverable and bondable but both
> > privacy modes seems to require the use of RPA, anyway it would
> > probably be a good idea to document the interactions, also main.conf
> > seem to have gone with privacy from the specification so we may either
> > need add more modes:
> >
> > Privacy:
> > "on"/"network": Apply Network Privacy Mode both for advertising and scanning
> > "limited-network": Apply Limited Discoverable Mode to advertising and
> > Network Privacy Mode for scanning
> > "device": Apply Device Privacy Mode both for advertising and scanning
> > "limited-device": Apply Limited Discoverable Mode to advertising and
> > Device Privacy Mode for scanning
>
> Then we can do that and use Limited Privacy mode for exactly that. However in addition I would use the Set Device Flags to allow punching holes for specific devices.

Yep, I actually realized that the reason we have limited privacy as a
mode for LE is that on dual-mode while discoverable it would already
blast the address over BR/EDR, so we might as well only allow the use
of limited-* modes on dual mode.

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz