Re: [Bluez PATCH v4 4/4] unit/test-gatt: Fix unknown request with signed bit

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Archie,

On Tue, Apr 7, 2020 at 1:56 AM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> The BT spec doesn't make it explicit of what should happen when
> receiving a bad signed att request packet.
>
> According to BT core spec Vol 3, Part C, Sec 10.4.2:
> A device receiving signed data shall authenticate it by performing
> the Signing Algorithm. If the MAC computed by the Signing Algorithm
> does not match the received MAC, the verification fails and the Host
> shall ignore the received Data PDU.

In my interpretation 'ignore the received Data PDU' may not supersed
the followin statement:

> According to BT core spec Vol 3, Part F, Sec 3.3
> If a server receives a request that it does not support, then the
> server shall respond with the ATT_ERROR_RSP PDU with the error code
> Request Not Supported.

So what I think we should be doing is to ignore the received data,
meaning it should not even be processed as a signed data since we
don't understand the command, _and_ respond with ATT_ERROR_RSP,
otherwise we risk having the 30 seconds timeout and disconnecting
since the request is no responded.

> This patch does this two things:
> (1) Removing the signed bit to the existing tests so they are not
>     in a conflicting state within the bluetooth spec, while still
>     keeping the original intent of the test.
> (2) Add another test that purposely fall within this grey area
>     with some comments.
> ---
>
> Changes in v4:
> - Fixing test-gatt.c
>
> Changes in v3: None
> Changes in v2: None
>
>  unit/test-gatt.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/unit/test-gatt.c b/unit/test-gatt.c
> index 36dd2847c..139a6fc72 100644
> --- a/unit/test-gatt.c
> +++ b/unit/test-gatt.c
> @@ -4473,16 +4473,38 @@ int main(int argc, char *argv[])
>                         raw_pdu(0x18, 0x01),
>                         raw_pdu(0x01, 0x18, 0x25, 0x00, 0x06));
>
> -       define_test_server("/robustness/unkown-request",
> +       define_test_server("/robustness/unknown-request",
>                         test_server, service_db_1, NULL,
>                         raw_pdu(0x03, 0x00, 0x02),
> -                       raw_pdu(0xbf, 0x00),
> -                       raw_pdu(0x01, 0xbf, 0x00, 0x00, 0x06));
> +                       raw_pdu(0x3f, 0x00),
> +                       raw_pdu(0x01, 0x3f, 0x00, 0x00, 0x06));

Afaik we used 0xbf because that was what PTS was using.

> +       define_test_server("/robustness/unknown-command",
> +                       test_server, service_db_1, NULL,
> +                       raw_pdu(0x03, 0x00, 0x02),
> +                       raw_pdu(0x7f, 0x00),
> +                       raw_pdu());
>
> -       define_test_server("/robustness/unkown-command",
> +       /*
> +        * According to BT core spec Vol 3, Part C, Sec 10.4.2:
> +        * A device receiving signed data shall authenticate it by performing
> +        * the Signing Algorithm. If the MAC computed by the Signing Algorithm
> +        * does not match the received MAC, the verification fails and the Host
> +        * shall ignore the received Data PDU.
> +        *
> +        * However, according to BT core spec Vol 3, Part F, Sec 3.3
> +        * If a server receives a request that it does not support, then the
> +        * server shall respond with the ATT_ERROR_RSP PDU with the error code
> +        * Request Not Supported.
> +        *
> +        * Since there is no explicit instruction on what should be done in
> +        * case the server receives a bad signed unsupported request, here
> +        * we just ignore the received PDU.
> +        */
> +       define_test_server("/robustness/signed-unknown-request",
>                         test_server, service_db_1, NULL,
>                         raw_pdu(0x03, 0x00, 0x02),
> -                       raw_pdu(0xff, 0x00),
> +                       raw_pdu(0xbf, 0x00),
>                         raw_pdu());
>
>         return tester_run();
> --
> 2.26.0.292.g33ef6b2f38-goog
>


-- 
Luiz Augusto von Dentz