Hi Frédéric,
On Tue, Jan 23, 2024 at 9:53 AM Frédéric Danis
<frederic.danis@xxxxxxxxxxxxx> wrote:
>
> Some PTS GATT tests like GATT/CL/GAR/BI-04-C request to be able to get the
> security error and do not try to change the security level.
>
> This commit adds the ability to prevent to change the security level for
> an operation.
> ---
> src/shared/att.c | 26 +++++++++++++
> src/shared/att.h | 1 +
> src/shared/gatt-client.c | 19 +++++++++
> src/shared/gatt-client.h | 3 ++
Please have the changes to src/shared in a separate patch.
> tools/btgatt-client.c | 84 ++++++++++++++++++++++++++++++++++------
> 5 files changed, 121 insertions(+), 12 deletions(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 62c884b65..485ef071b 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -2042,3 +2042,29 @@ bool bt_att_has_crypto(struct bt_att *att)
>
> return att->crypto ? true : false;
> }
> +
> +bool bt_att_set_retry(struct bt_att *att, unsigned int id, bool retry)
> +{
> + struct att_send_op *op;
> +
> + if (!id)
> + return false;
> +
> + op = queue_find(att->req_queue, match_op_id, UINT_TO_PTR(id));
> + if (op)
> + goto done;
> +
> + op = queue_find(att->ind_queue, match_op_id, UINT_TO_PTR(id));
> + if (op)
> + goto done;
> +
> + op = queue_find(att->write_queue, match_op_id, UINT_TO_PTR(id));
> +
> +done:
> + if (!op)
> + return false;
> +
> + op->retry = !retry;
> +
> + return true;
> +}
> diff --git a/src/shared/att.h b/src/shared/att.h
> index 4aa3de87b..6fd78636e 100644
> --- a/src/shared/att.h
> +++ b/src/shared/att.h
> @@ -110,3 +110,4 @@ bool bt_att_set_local_key(struct bt_att *att, uint8_t sign_key[16],
> bool bt_att_set_remote_key(struct bt_att *att, uint8_t sign_key[16],
> bt_att_counter_func_t func, void *user_data);
> bool bt_att_has_crypto(struct bt_att *att);
> +bool bt_att_set_retry(struct bt_att *att, unsigned int id, bool retry);
> diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
> index 5de679c9b..6340bcd85 100644
> --- a/src/shared/gatt-client.c
> +++ b/src/shared/gatt-client.c
> @@ -3818,3 +3818,22 @@ bool bt_gatt_client_idle_unregister(struct bt_gatt_client *client,
>
> return false;
> }
> +
> +bool bt_gatt_client_set_retry(struct bt_gatt_client *client,
> + unsigned int id,
> + bool retry)
> +{
> + struct request *req;
> +
> + if (!client || !id)
> + return false;
> +
> + req = queue_find(client->pending_requests, match_req_id,
> + UINT_TO_PTR(id));
> + if (!req)
> + return false;
> +
> + bt_att_set_retry(client->att, req->att_id, retry);
> +
> + return true;
> +}
> diff --git a/src/shared/gatt-client.h b/src/shared/gatt-client.h
> index bccd04a62..63cf99500 100644
> --- a/src/shared/gatt-client.h
> +++ b/src/shared/gatt-client.h
> @@ -134,3 +134,6 @@ unsigned int bt_gatt_client_idle_register(struct bt_gatt_client *client,
> bt_gatt_client_destroy_func_t destroy);
> bool bt_gatt_client_idle_unregister(struct bt_gatt_client *client,
> unsigned int id);
> +bool bt_gatt_client_set_retry(struct bt_gatt_client *client,
> + unsigned int id,
> + bool retry);
> diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
> index 58a03bd48..3bcb7e1cf 100644
> --- a/tools/btgatt-client.c
> +++ b/tools/btgatt-client.c
> @@ -57,6 +57,7 @@ struct client {
> struct bt_gatt_client *gatt;
>
> unsigned int reliable_session_id;
> + bool sec_retry;
> };
>
> static void print_prompt(void)
> @@ -172,6 +173,7 @@ static struct client *client_create(int fd, uint16_t mtu)
> fprintf(stderr, "Failed to allocate memory for client\n");
> return NULL;
> }
> + cli->sec_retry = true;
>
> cli->att = bt_att_new(fd, false);
> if (!cli->att) {
> @@ -488,6 +490,7 @@ static void cmd_read_multiple(struct client *cli, char *cmd_str)
> char *argv[512];
> int i;
> char *endptr = NULL;
> + unsigned int id;
>
> if (!bt_gatt_client_is_ready(cli->gatt)) {
> printf("GATT client not initialized\n");
> @@ -514,9 +517,12 @@ static void cmd_read_multiple(struct client *cli, char *cmd_str)
> }
> }
>
> - if (!bt_gatt_client_read_multiple(cli->gatt, value, argc,
> - read_multiple_cb, NULL, NULL))
> + id = bt_gatt_client_read_multiple(cli->gatt, value, argc,
> + read_multiple_cb, NULL, NULL);
> + if (!id)
> printf("Failed to initiate read multiple procedure\n");
> + else if (!cli->sec_retry)
> + bt_gatt_client_set_retry(cli->gatt, id, false);
>
> free(value);
> }
> @@ -558,6 +564,7 @@ static void cmd_read_value(struct client *cli, char *cmd_str)
> int argc = 0;
> uint16_t handle;
> char *endptr = NULL;
> + unsigned int id;
>
> if (!bt_gatt_client_is_ready(cli->gatt)) {
> printf("GATT client not initialized\n");
> @@ -575,9 +582,12 @@ static void cmd_read_value(struct client *cli, char *cmd_str)
> return;
> }
>
> - if (!bt_gatt_client_read_value(cli->gatt, handle, read_cb,
> - NULL, NULL))
> + id = bt_gatt_client_read_value(cli->gatt, handle, read_cb,
> + NULL, NULL);
> + if (!id)
> printf("Failed to initiate read value procedure\n");
> + else if (!cli->sec_retry)
> + bt_gatt_client_set_retry(cli->gatt, id, false);
> }
>
> static void read_long_value_usage(void)
> @@ -592,6 +602,7 @@ static void cmd_read_long_value(struct client *cli, char *cmd_str)
> uint16_t handle;
> uint16_t offset;
> char *endptr = NULL;
> + unsigned int id;
>
> if (!bt_gatt_client_is_ready(cli->gatt)) {
> printf("GATT client not initialized\n");
> @@ -616,9 +627,12 @@ static void cmd_read_long_value(struct client *cli, char *cmd_str)
> return;
> }
>
> - if (!bt_gatt_client_read_long_value(cli->gatt, handle, offset, read_cb,
> - NULL, NULL))
> + id = bt_gatt_client_read_long_value(cli->gatt, handle, offset, read_cb,
> + NULL, NULL);
> + if (!id)
> printf("Failed to initiate read long value procedure\n");
> + else if (!cli->sec_retry)
> + bt_gatt_client_set_retry(cli->gatt, id, false);
> }
>
> static void write_value_usage(void)
> @@ -659,6 +673,7 @@ static void cmd_write_value(struct client *cli, char *cmd_str)
> uint8_t *value = NULL;
> bool without_response = false;
> bool signed_write = false;
> + unsigned int id;
>
> if (!bt_gatt_client_is_ready(cli->gatt)) {
> printf("GATT client not initialized\n");
> @@ -740,10 +755,13 @@ static void cmd_write_value(struct client *cli, char *cmd_str)
> goto done;
> }
>
> - if (!bt_gatt_client_write_value(cli->gatt, handle, value, length,
> + id = bt_gatt_client_write_value(cli->gatt, handle, value, length,
> write_cb,
> - NULL, NULL))
> + NULL, NULL);
> + if (!id)
> printf("Failed to initiate write procedure\n");
> + else if (!cli->sec_retry)
> + bt_gatt_client_set_retry(cli->gatt, id, false);
>
> done:
> free(value);
> @@ -789,6 +807,7 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
> int length;
> uint8_t *value = NULL;
> bool reliable_writes = false;
> + unsigned int id;
>
> if (!bt_gatt_client_is_ready(cli->gatt)) {
> printf("GATT client not initialized\n");
> @@ -863,11 +882,14 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
> }
> }
>
> - if (!bt_gatt_client_write_long_value(cli->gatt, reliable_writes, handle,
> + id = bt_gatt_client_write_long_value(cli->gatt, reliable_writes, handle,
> offset, value, length,
> write_long_cb,
> - NULL, NULL))
> + NULL, NULL);
> + if (!id)
> printf("Failed to initiate long write procedure\n");
> + else if (!cli->sec_retry)
> + bt_gatt_client_set_retry(cli->gatt, id, false);
>
> free(value);
> }
> @@ -999,12 +1021,18 @@ done:
> value, length,
> write_long_cb, NULL,
> NULL);
> - if (!cli->reliable_session_id)
> + if (!cli->reliable_session_id) {
> printf("Failed to proceed prepare write\n");
> - else
> + } else {
> + if (!cli->sec_retry)
> + bt_gatt_client_set_retry(cli->gatt,
> + cli->reliable_session_id,
> + false);
> +
> printf("Prepare write success.\n"
> "Session id: %d to be used on next write\n",
> cli->reliable_session_id);
> + }
>
> free(value);
> }
> @@ -1236,6 +1264,36 @@ static void cmd_get_security(struct client *cli, char *cmd_str)
> printf("Security level: %u\n", level);
> }
>
> +static void set_security_retry_usage(void)
> +{
> + printf("Usage: set-security-retry <y/n>\n"
> + "e.g.:\n"
> + "\tset-security-retry n\n");
> +}
> +
> +static void cmd_set_security_retry(struct client *cli, char *cmd_str)
> +{
> + char *argv[2];
> + int argc = 0;
> +
> + if (!bt_gatt_client_is_ready(cli->gatt)) {
> + printf("GATT client not initialized\n");
> + return;
> + }
> +
> + if (!parse_args(cmd_str, 1, argv, &argc) || argc != 1) {
> + set_security_retry_usage();
> + return;
> + }
> +
> + if (argv[0][0] == 'y')
> + cli->sec_retry = true;
> + else if (argv[0][0] == 'n')
> + cli->sec_retry = false;
> + else
> + printf("Invalid argument: %s\n", argv[0]);
> +}
> +
> static bool convert_sign_key(char *optarg, uint8_t key[16])
> {
> int i;
> @@ -1327,6 +1385,8 @@ static struct {
> "\tSet security level on le connection"},
> { "get-security", cmd_get_security,
> "\tGet security level on le connection"},
> + { "set-security-retry", cmd_set_security_retry,
> + "\tSet retry on security error by elevating security"},
> { "set-sign-key", cmd_set_sign_key,
> "\tSet signing key for signed write command"},
> { }
> --
> 2.34.1
>
>
--
Luiz Augusto von Dentz
