Re: [PATCH v2 1/3] eir: parse data types for LE OOB pairing

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

Hi Michael,

On Fri, Jun 10, 2022 at 3:49 PM Michael Brudevold <puffy.taco@gmail.com> wrote:
>
> From: Michael Brudevold <michael.brudevold@veranexsolutions.com>
>
> LE support added for P-256 and split out from existing BREDR support for
> P-192
>
> Also attempt to free any existing values before setting new values
> ---
>  plugins/neard.c |  8 ++++----
>  src/eir.c       | 41 +++++++++++++++++++++++++++++++++++------
>  src/eir.h       | 10 ++++++++--
>  3 files changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/plugins/neard.c b/plugins/neard.c
> index 99762482c..11d9e91c4 100644
> --- a/plugins/neard.c
> +++ b/plugins/neard.c
> @@ -352,11 +352,11 @@ static int process_eir(uint8_t *eir, size_t size, struct oob_params *remote)
>         remote->services = eir_data.services;
>         eir_data.services = NULL;
>
> -       remote->hash = eir_data.hash;
> -       eir_data.hash = NULL;
> +       remote->hash = eir_data.hash192;
> +       eir_data.hash192 = NULL;
>
> -       remote->randomizer = eir_data.randomizer;
> -       eir_data.randomizer = NULL;
> +       remote->randomizer = eir_data.randomizer192;
> +       eir_data.randomizer192 = NULL;
>
>         eir_data_free(&eir_data);
>
> diff --git a/src/eir.c b/src/eir.c
> index 2f9ee036f..79d423074 100644
> --- a/src/eir.c
> +++ b/src/eir.c
> @@ -53,10 +53,14 @@ void eir_data_free(struct eir_data *eir)
>         eir->services = NULL;
>         g_free(eir->name);
>         eir->name = NULL;
> -       free(eir->hash);
> -       eir->hash = NULL;
> -       free(eir->randomizer);
> -       eir->randomizer = NULL;
> +       free(eir->hash192);
> +       eir->hash192 = NULL;
> +       free(eir->randomizer192);
> +       eir->randomizer192 = NULL;
> +       free(eir->hash256);
> +       eir->hash256 = NULL;
> +       free(eir->randomizer256);
> +       eir->randomizer256 = NULL;
>         g_slist_free_full(eir->msd_list, g_free);
>         eir->msd_list = NULL;
>         g_slist_free_full(eir->sd_list, sd_free);
> @@ -323,13 +327,15 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
>                 case EIR_SSP_HASH:
>                         if (data_len < 16)
>                                 break;
> -                       eir->hash = util_memdup(data, 16);
> +                       free(eir->hash192);
> +                       eir->hash192 = util_memdup(data, 16);
>                         break;
>
>                 case EIR_SSP_RANDOMIZER:
>                         if (data_len < 16)
>                                 break;
> -                       eir->randomizer = util_memdup(data, 16);
> +                       free(eir->randomizer192);
> +                       eir->randomizer192 = util_memdup(data, 16);
>                         break;
>
>                 case EIR_DEVICE_ID:
> @@ -342,6 +348,15 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
>                         eir->did_version = data[6] | (data[7] << 8);
>                         break;
>
> +               case EIR_LE_DEVICE_ADDRESS:
> +                       if (data_len < sizeof(bdaddr_t) + 1)
> +                               break;
> +
> +                       memcpy(&eir->addr, data, sizeof(bdaddr_t));
> +                       eir->addr_type = data[sizeof(bdaddr_t)] & 0x1 ?
> +                                       BDADDR_LE_RANDOM : BDADDR_LE_PUBLIC;
> +                       break;
> +
>                 case EIR_SVC_DATA16:
>                         eir_parse_uuid16_data(eir, data, data_len);
>                         break;
> @@ -354,6 +369,20 @@ void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
>                         eir_parse_uuid128_data(eir, data, data_len);
>                         break;
>
> +               case EIR_LE_SC_CONF:
> +                       if (data_len < 16)
> +                               break;
> +                       free(eir->hash256);
> +                       eir->hash256 = util_memdup(data, 16);
> +                       break;
> +
> +               case EIR_LE_SC_RAND:
> +                       if (data_len < 16)
> +                               break;
> +                       free(eir->randomizer256);
> +                       eir->randomizer256 = util_memdup(data, 16);
> +                       break;
> +
>                 case EIR_MANUFACTURER_DATA:
>                         eir_parse_msd(eir, data, data_len);
>                         break;
> diff --git a/src/eir.h b/src/eir.h
> index 6154e23ec..b2cf00f37 100644
> --- a/src/eir.h
> +++ b/src/eir.h
> @@ -33,9 +33,12 @@
>  #define EIR_PUB_TRGT_ADDR           0x17  /* LE: Public Target Address */
>  #define EIR_RND_TRGT_ADDR           0x18  /* LE: Random Target Address */
>  #define EIR_GAP_APPEARANCE          0x19  /* GAP appearance */
> +#define EIR_LE_DEVICE_ADDRESS       0x1B  /* LE: Bluetooth Device Address */
>  #define EIR_SOLICIT32               0x1F  /* LE: Solicit UUIDs, 32-bit */
>  #define EIR_SVC_DATA32              0x20  /* LE: Service data, 32-bit UUID */
>  #define EIR_SVC_DATA128             0x21  /* LE: Service data, 128-bit UUID */
> +#define EIR_LE_SC_CONF              0x22  /* LE: Secure Connections Confirmation Value */
> +#define EIR_LE_SC_RAND              0x23  /* LE: Secure Connections Random Value */
>  #define EIR_TRANSPORT_DISCOVERY     0x26  /* Transport Discovery Service */
>  #define EIR_MANUFACTURER_DATA       0xFF  /* Manufacturer Specific Data */
>
> @@ -77,9 +80,12 @@ struct eir_data {
>         uint16_t appearance;
>         bool name_complete;
>         int8_t tx_power;
> -       uint8_t *hash;
> -       uint8_t *randomizer;
> +       uint8_t *hash192;
> +       uint8_t *randomizer192;
> +       uint8_t *hash256;
> +       uint8_t *randomizer256;
>         bdaddr_t addr;
> +       uint8_t addr_type;
>         uint16_t did_vendor;
>         uint16_t did_product;
>         uint16_t did_version;
> --
> 2.25.1

It might be better to handle this via bt_ad instance instead of
eir_data, in fact the plan was always to switch to bt_ad but it seems
we forgot about it at some point.



-- 
Luiz Augusto von Dentz