[PATCH v2 1/3] shared/att: Respond to any opcode that is not command

[PATCH v2 1/3] shared/att: Respond to any opcode that is not command

[Luiz Augusto von Dentz]

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

Although get_op_type does now check for the command mask it must
respond to anything other than ATT_OP_TYPE_CMD since there is no mask
for response opcode.
---
 src/shared/att.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/shared/att.c b/src/shared/att.c
index 494b10d..ca2d051 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -149,7 +149,7 @@ static enum att_op_type get_op_type(uint8_t opcode)
 	}
 
 	if (opcode & ATT_OP_CMD_MASK)
-		return ATT_OP_CMD_MASK;
+		return ATT_OP_TYPE_CMD;
 
 	return ATT_OP_TYPE_UNKNOWN;
 }
@@ -841,10 +841,10 @@ static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
 	}
 
 	/*
-	 * If this was a request and no handler was registered for it, respond
-	 * with "Not Supported"
+	 * If this was not a command and no handler was registered for it,
+	 * respond with "Not Supported"
 	 */
-	if (!found && get_op_type(opcode) == ATT_OP_TYPE_REQ)
+	if (!found && get_op_type(opcode) != ATT_OP_TYPE_CMD)
 		respond_not_supported(att, opcode);
 
 	bt_att_unref(att);
-- 
2.9.3

[PATCH v2 2/3] unit/test-gatt: Add test for unknown request

[Luiz Augusto von Dentz]

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

This send an unknown request (0xbf) to test if a response is sent since
the spec mandates the server to always respond to all requests it
receives:

BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part F page 2173

  'A client may send attribute protocol requests to a server, and the
  server shall respond to all requests that it receives.'
---
 unit/test-gatt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index c7a8fa5..371cb46 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -4461,5 +4461,11 @@ int main(int argc, char *argv[])
 			raw_pdu(0x18, 0x01),
 			raw_pdu(0x01, 0x18, 0x25, 0x00, 0x06));
 
+	define_test_server("/robustness/unkown-request",
+			test_server, service_db_1, NULL,
+			raw_pdu(0x03, 0x00, 0x02),
+			raw_pdu(0xbf, 0x00),
+			raw_pdu(0x01, 0xbf, 0x00, 0x00, 0x06));
+
 	return tester_run();
 }
-- 
2.9.3

[PATCH v2 3/3] unit/test-gatt: Add test for unknown request

[Luiz Augusto von Dentz]

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

This send an unknown command (0xff) to test that no response is sent
since the spec mandates the server to not respond to commands:

BLUETOOTH SPECIFICATION Version 5.0 | Vol 3, Part F page 2179

  'If a server receives a command that it does not support, indicated
  by the Command Flag of the PDU set to one, then the server shall
  ignore the Command.'
---
 unit/test-gatt.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/unit/test-gatt.c b/unit/test-gatt.c
index 371cb46..e0d29c7 100644
--- a/unit/test-gatt.c
+++ b/unit/test-gatt.c
@@ -90,6 +90,11 @@ struct context {
 		.size = sizeof(data(args)),			\
 	}
 
+#define false_pdu()						\
+	{							\
+		.valid = false,					\
+	}
+
 #define define_test(name, function, type, bt_uuid, db,			\
 		test_step, args...)					\
 	do {								\
@@ -403,6 +408,13 @@ static gboolean send_pdu(gpointer user_data)
 	if (pdu->valid && (pdu->size == 0)) {
 		test_debug("(no action expected)", "GATT: ");
 		context->pdu_offset++;
+
+		/* Quit the context if we processed the last PDU */
+		if (!context->data->pdu_list[context->pdu_offset].valid) {
+			context_quit(context);
+			return FALSE;
+		}
+
 		return send_pdu(context);
 	}
 
@@ -4467,5 +4479,11 @@ int main(int argc, char *argv[])
 			raw_pdu(0xbf, 0x00),
 			raw_pdu(0x01, 0xbf, 0x00, 0x00, 0x06));
 
+	define_test_server("/robustness/unkown-command",
+			test_server, service_db_1, NULL,
+			raw_pdu(0x03, 0x00, 0x02),
+			raw_pdu(0xff, 0x00),
+			raw_pdu());
+
 	return tester_run();
 }
-- 
2.9.3

Re: [PATCH v2 1/3] shared/att: Respond to any opcode that is not command

[Luiz Augusto von Dentz]

Hi,

On Mon, May 1, 2017 at 6:56 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> Although get_op_type does now check for the command mask it must
> respond to anything other than ATT_OP_TYPE_CMD since there is no mask
> for response opcode.
> ---
>  src/shared/att.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 494b10d..ca2d051 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -149,7 +149,7 @@ static enum att_op_type get_op_type(uint8_t opcode)
>         }
>
>         if (opcode & ATT_OP_CMD_MASK)
> -               return ATT_OP_CMD_MASK;
> +               return ATT_OP_TYPE_CMD;
>
>         return ATT_OP_TYPE_UNKNOWN;
>  }
> @@ -841,10 +841,10 @@ static void handle_notify(struct bt_att *att, uint8_t opcode, uint8_t *pdu,
>         }
>
>         /*
> -        * If this was a request and no handler was registered for it, respond
> -        * with "Not Supported"
> +        * If this was not a command and no handler was registered for it,
> +        * respond with "Not Supported"
>          */
> -       if (!found && get_op_type(opcode) == ATT_OP_TYPE_REQ)
> +       if (!found && get_op_type(opcode) != ATT_OP_TYPE_CMD)
>                 respond_not_supported(att, opcode);
>
>         bt_att_unref(att);
> --
> 2.9.3

Applied.


-- 
Luiz Augusto von Dentz