2016-03-02 13:28:39

by Joseph Hwang

[permalink] [raw]
Subject: [PATCH] shared/att: change security as needed

From: Joseph Hwang <[email protected]>

When pairing with a BLE keyboard, bluetoothd suffers from
authentication errors as follows:

ERR bluetoothd[1103]: Report Map read failed:
Attribute requires authentication before read/write
ERR bluetoothd[1103]: Protocol Mode characteristic read failed:
Attribute requires authentication before read/write
ERR bluetoothd[1103]: HID Information read failed:
Attribute requires authentication before read/write

This is because the original security level is BT_ATT_SECURITY_LOW,
while BT_ATT_SECURITY_HIGH is required for pairing. This patch enables
the security elevation so that handle_error_rsp() could push the
operation back to request queue properly.

---
src/shared/att.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index 3a84783..331fae7 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -574,9 +574,6 @@ static bool change_security(struct bt_att *att, uint8_t ecode)
int security;

security = bt_att_get_security(att);
- if (security != BT_ATT_SECURITY_AUTO)
- return false;
-
if (ecode == BT_ATT_ERROR_INSUFFICIENT_ENCRYPTION &&
security < BT_ATT_SECURITY_MEDIUM)
security = BT_ATT_SECURITY_MEDIUM;
--
2.7.0.rc3.207.g0ac5344



2016-03-02 14:04:11

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] shared/att: change security as needed

Hi Joseph,

On Wed, Mar 2, 2016 at 3:55 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Joseph,
>
> On Wed, Mar 2, 2016 at 3:28 PM, <[email protected]> wrote:
>> From: Joseph Hwang <[email protected]>
>>
>> When pairing with a BLE keyboard, bluetoothd suffers from
>> authentication errors as follows:
>>
>> ERR bluetoothd[1103]: Report Map read failed:
>> Attribute requires authentication before read/write
>> ERR bluetoothd[1103]: Protocol Mode characteristic read failed:
>> Attribute requires authentication before read/write
>> ERR bluetoothd[1103]: HID Information read failed:
>> Attribute requires authentication before read/write
>>
>> This is because the original security level is BT_ATT_SECURITY_LOW,
>> while BT_ATT_SECURITY_HIGH is required for pairing. This patch enables
>> the security elevation so that handle_error_rsp() could push the
>> operation back to request queue properly.
>>
>> ---
>> src/shared/att.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/src/shared/att.c b/src/shared/att.c
>> index 3a84783..331fae7 100644
>> --- a/src/shared/att.c
>> +++ b/src/shared/att.c
>> @@ -574,9 +574,6 @@ static bool change_security(struct bt_att *att, uint8_t ecode)
>> int security;
>>
>> security = bt_att_get_security(att);
>> - if (security != BT_ATT_SECURITY_AUTO)
>> - return false;
>> -
>
> While this perhaps works it seems to be just a work around the
> problem, it seems to me that ATT security shall be set to
> BT_ATT_SECURITY_AUTO not to BT_ATT_SECURITY_LOW which is preventing
> security to be elevated.

Something like this should fix it:

index 14e850e..6bc44b6 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4710,6 +4710,7 @@ bool device_attach_att(struct btd_device *dev,
GIOChannel *io)
dev->att = g_attrib_get_att(attrib);

bt_att_ref(dev->att);
+ bt_att_set_security(dev->att, BT_ATT_SECURITY_AUTO);

dev->att_disconn_id = bt_att_register_disconnect(dev->att,
att_disconnected_cb, dev, NULL);

--
Luiz Augusto von Dentz

2016-03-02 13:55:46

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] shared/att: change security as needed

Hi Joseph,

On Wed, Mar 2, 2016 at 3:28 PM, <[email protected]> wrote:
> From: Joseph Hwang <[email protected]>
>
> When pairing with a BLE keyboard, bluetoothd suffers from
> authentication errors as follows:
>
> ERR bluetoothd[1103]: Report Map read failed:
> Attribute requires authentication before read/write
> ERR bluetoothd[1103]: Protocol Mode characteristic read failed:
> Attribute requires authentication before read/write
> ERR bluetoothd[1103]: HID Information read failed:
> Attribute requires authentication before read/write
>
> This is because the original security level is BT_ATT_SECURITY_LOW,
> while BT_ATT_SECURITY_HIGH is required for pairing. This patch enables
> the security elevation so that handle_error_rsp() could push the
> operation back to request queue properly.
>
> ---
> src/shared/att.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 3a84783..331fae7 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -574,9 +574,6 @@ static bool change_security(struct bt_att *att, uint8_t ecode)
> int security;
>
> security = bt_att_get_security(att);
> - if (security != BT_ATT_SECURITY_AUTO)
> - return false;
> -

While this perhaps works it seems to be just a work around the
problem, it seems to me that ATT security shall be set to
BT_ATT_SECURITY_AUTO not to BT_ATT_SECURITY_LOW which is preventing
security to be elevated.

--
Luiz Augusto von Dentz