Subject: GATT server does not handle "Invalid Offset" and "Invalid Attribute Value Length" errors properly

Hi,

According to Bluetooth specs (BLUETOOTH CORE SPECIFICATION Version 5.1
| Vol 3, Part F 3.4.6.1 Prepare Write Request):

"... The Attribute Value validation is done when an Execute Write
Request is received. Hence, any Invalid Offset or Invalid Attribute
Value Length errors are generated when an Execute Write Request is
received. ..."

In contrary to the specs, 'bluetoothd' is sending error response during
prepare write. The following patch changes this behaviour:


--- a/src/shared/gatt-server.c 2018-06-01 10:37:36.000000000 +0200
+++ b/src/shared/gatt-server.c 2019-12-13 12:25:22.000000000 +0100
@@ -1223,7 +1223,17 @@

handle = get_le16(pwcd->pdu);

- if (err) {
+ if ((0 != err) && (BT_ATT_ERROR_INVALID_OFFSET != err)
+ && (BT_ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN != err)) {
bt_att_send_error_rsp(pwcd->server->att,
BT_ATT_OP_PREP_WRITE_REQ,
handle, err);
free(pwcd->pdu);

Best regards,
Konstantin


2019-12-19 22:14:35

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: GATT server does not handle "Invalid Offset" and "Invalid Attribute Value Length" errors properly

Hi Konstantin,

On Thu, Dec 19, 2019 at 12:36 AM Konstantin Forostyan
<[email protected]> wrote:
>
> Hi,
>
> According to Bluetooth specs (BLUETOOTH CORE SPECIFICATION Version 5.1
> | Vol 3, Part F 3.4.6.1 Prepare Write Request):
>
> "... The Attribute Value validation is done when an Execute Write
> Request is received. Hence, any Invalid Offset or Invalid Attribute
> Value Length errors are generated when an Execute Write Request is
> received. ..."
>
> In contrary to the specs, 'bluetoothd' is sending error response during
> prepare write. The following patch changes this behaviour:
>
>
> --- a/src/shared/gatt-server.c 2018-06-01 10:37:36.000000000 +0200
> +++ b/src/shared/gatt-server.c 2019-12-13 12:25:22.000000000 +0100
> @@ -1223,7 +1223,17 @@
>
> handle = get_le16(pwcd->pdu);
>
> - if (err) {
> + if ((0 != err) && (BT_ATT_ERROR_INVALID_OFFSET != err)
> + && (BT_ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN != err)) {
> bt_att_send_error_rsp(pwcd->server->att,
> BT_ATT_OP_PREP_WRITE_REQ,
> handle, err);
> free(pwcd->pdu);

Yep, I remember this one actually since we did fix something similar
in Zephyr we would need to move the error checking to execute, btw is
this with bluetoothd or gatt-server tool, the later is probably not
recommended for qualification as it is more of a validation tool it
may not be feature complete.


--
Luiz Augusto von Dentz

Subject: Re: GATT server does not handle "Invalid Offset" and "Invalid Attribute Value Length" errors properly

Hi Luiz,

> Yep, I remember this one actually since we did fix something similar
> in Zephyr we would need to move the error checking to execute, btw is
Thank you, I'll take a look on that.

> this with bluetoothd or gatt-server tool,
This is in bluetoothd.

> gatt-server tool, the later is probably not
> recommended for qualification as it is more of a validation tool it
> may not be feature complete.
Could you please tell me what is recommended for qualification?

Thank you in advance!
Konstantin


On Thu, 2019-12-19 at 14:12 -0800, Luiz Augusto von Dentz wrote:
> Hi Konstantin,
>
> On Thu, Dec 19, 2019 at 12:36 AM Konstantin Forostyan
> <
> [email protected]
> > wrote:
> > Hi,
> >
> > According to Bluetooth specs (BLUETOOTH CORE SPECIFICATION Version
> > 5.1
> > > Vol 3, Part F 3.4.6.1 Prepare Write Request):
> >
> > "... The Attribute Value validation is done when an Execute Write
> > Request is received. Hence, any Invalid Offset or Invalid Attribute
> > Value Length errors are generated when an Execute Write Request is
> > received. ..."
> >
> > In contrary to the specs, 'bluetoothd' is sending error response
> > during
> > prepare write. The following patch changes this behaviour:
> >
> >
> > --- a/src/shared/gatt-server.c 2018-06-01 10:37:36.000000000 +0200
> > +++ b/src/shared/gatt-server.c 2019-12-13 12:25:22.000000000 +0100
> > @@ -1223,7 +1223,17 @@
> >
> > handle = get_le16(pwcd->pdu);
> >
> > - if (err) {
> > + if ((0 != err) && (BT_ATT_ERROR_INVALID_OFFSET != err)
> > + && (BT_ATT_ERROR_INVALID_ATTRIBUTE_VALUE_LEN !=
> > err)) {
> > bt_att_send_error_rsp(pwcd->server->att,
> > BT_ATT_OP_PREP_WRITE_REQ,
> > handle, err);
> > free(pwcd->pdu);
>
> Yep, I remember this one actually since we did fix something similar
> in Zephyr we would need to move the error checking to execute, btw is
> this with bluetoothd or gatt-server tool, the later is probably not
> recommended for qualification as it is more of a validation tool it
> may not be feature complete.
>
>