Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221122101232.45320-1-sathish.narasimman@intel.com>
 <20221122101232.45320-4-sathish.narasimman@intel.com> <CABBYNZKsRV4W63KRrsJ7Ay8Er0NBV1DGEWqtgceoTaaA2VowQA@mail.gmail.com>
In-Reply-To: <CABBYNZKsRV4W63KRrsJ7Ay8Er0NBV1DGEWqtgceoTaaA2VowQA@mail.gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 22 Nov 2022 16:07:47 -0800
Message-ID: <CABBYNZJ5YqJJ1wu00PCszxPdgSZOvKv1Uy9DKfgDObum+AnKWw@mail.gmail.com>
Subject: Re: [PATCH BlueZ v3 3/7] main.conf: Add CSIP profile configurable options
To:     Sathish Narasimman <sathish.narasimman@intel.com>
Cc:     linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Sathish,

On Tue, Nov 22, 2022 at 12:13 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sathish,
>
>
> On Tue, Nov 22, 2022 at 2:17 AM Sathish Narasimman
> <sathish.narasimman@intel.com> wrote:
> >
> > This introduces option to configure main.conf that can be used to
> > configure co-ordinated set identification profile.
> > ---
> >  src/btd.h     |   9 ++++
> >  src/main.c    | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/main.conf |  24 +++++++++++
> >  3 files changed, 146 insertions(+)
> >
> > diff --git a/src/btd.h b/src/btd.h
> > index 42cffcde43ca..a3683a098689 100644
> > --- a/src/btd.h
> > +++ b/src/btd.h
> > @@ -92,6 +92,13 @@ struct btd_defaults {
> >         struct btd_le_defaults le;
> >  };
> >
> > +struct btd_csis {
> > +       uint8_t sirk_type;
> > +       uint8_t sirk_val[16];
> > +       uint8_t cs_size;
> > +       uint8_t cs_rank;
> > +};
> > +
> >  struct btd_avdtp_opts {
> >         uint8_t  session_mode;
> >         uint8_t  stream_mode;
> > @@ -142,6 +149,8 @@ struct btd_opts {
> >         enum jw_repairing_t jw_repairing;
> >
> >         struct btd_advmon_opts  advmon;
> > +
> > +       struct btd_csis csis_defaults;
> >  };
> >
> >  extern struct btd_opts btd_opts;
> > diff --git a/src/main.c b/src/main.c
> > index 99d9c508ff91..abb422961f78 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -60,6 +60,9 @@
> >  #define DEFAULT_TEMPORARY_TIMEOUT         30 /* 30 seconds */
> >  #define DEFAULT_NAME_REQUEST_RETRY_DELAY 300 /* 5 minutes */
> >
> > +/*CSIP Profile - Server */
> > +#define DEFAULT_SIRK "761FAE703ED681F0C50B34155B6434FB"
> > +
> >  #define SHUTDOWN_GRACE_SECONDS 10
> >
> >  struct btd_opts btd_opts;
> > @@ -146,6 +149,14 @@ static const char *gatt_options[] = {
> >         NULL
> >  };
> >
> > +static const char *csip_options[] = {
> > +       "CsisSirkType",
> > +       "CsisSirkValue",
> > +       "CsisSize",
> > +       "CsisRank",
> > +       NULL
> > +};
> > +
> >  static const char *avdtp_options[] = {
> >         "SessionMode",
> >         "StreamMode",
> > @@ -166,11 +177,55 @@ static const struct group_table {
> >         { "LE",         le_options },
> >         { "Policy",     policy_options },
> >         { "GATT",       gatt_options },
> > +       { "CSIP",       csip_options },
> >         { "AVDTP",      avdtp_options },
> >         { "AdvMon",     advmon_options },
> >         { }
> >  };
> >
> > +#ifndef MIN
> > +#define MIN(x, y) ((x) < (y) ? (x) : (y))
> > +#endif
> > +
> > +static int8_t check_sirk_alpha_numeric(char *str)
> > +{
> > +       int8_t val = 0;
> > +       char *s = str;
> > +
> > +       if (strlen(s) != 32) /* 32 Bytes of Alpha numeric string */
> > +               return 0;
> > +
> > +       for ( ; *s; s++) {
> > +               if (((*s >= '0') & (*s <= '9'))
> > +                       || ((*s >= 'a') && (*s <= 'z'))
> > +                       || ((*s >= 'A') && (*s <= 'Z'))) {
> > +                       val = 1;
> > +               } else {
> > +                       val = 0;
> > +                       break;
> > +               }
> > +       }
> > +
> > +       return val;
> > +}
> > +
> > +static size_t hex2bin(const char *hexstr, uint8_t *buf, size_t buflen)
> > +{
> > +       size_t i, len;
> > +
> > +       if (!hexstr)
> > +               return 0;
> > +
> > +       len = MIN((strlen(hexstr) / 2), buflen);
> > +       memset(buf, 0, len);
> > +
> > +       for (i = 0; i < len; i++) {
> > +               if (sscanf(hexstr + (i * 2), "%02hhX", &buf[i]) != 1)
> > +                       continue;
> > +       }
> > +
> > +       return len;
> > +}
> >
> >  GKeyFile *btd_get_main_conf(void)
> >  {
> > @@ -939,6 +994,58 @@ static void parse_config(GKeyFile *config)
> >                 btd_opts.gatt_channels = val;
> >         }
> >
> > +       val = g_key_file_get_integer(config, "CSIP", "CsisSirkType", &err);
> > +       if (err) {
> > +               DBG("%s", err->message);
> > +               g_clear_error(&err);
> > +       } else {
> > +               val = MIN(val, 2);
> > +               val = MAX(val, 1);
> > +               DBG("Csis Type: %u", val);
> > +               btd_opts.csis_defaults.cs_size = val;
> > +       }
> > +
> > +       str = g_key_file_get_string(config, "CSIP", "CsisSirkValue", &err);
> > +       if (err) {
> > +               DBG("%s", err->message);
> > +               g_clear_error(&err);
> > +       } else {
> > +               DBG("Csis Sirk: %s", str);
> > +
> > +               if (!check_sirk_alpha_numeric(str)) {
> > +                       DBG("SIRK is not apha numeric Value");
> > +                       return;
> > +               }
> > +
> > +               btd_opts.csis_defaults.sirk_type = 1; /* Plain Text - Type*/
> > +               hex2bin(str, btd_opts.csis_defaults.sirk_val,
> > +                       sizeof(btd_opts.csis_defaults.sirk_val));
> > +
> > +               g_free(str);
> > +       }
> > +
> > +       val = g_key_file_get_integer(config, "CSIP", "CsisSize", &err);
> > +       if (err) {
> > +               DBG("%s", err->message);
> > +               g_clear_error(&err);
> > +       } else {
> > +               val = MIN(val, 0xFF);
> > +               val = MAX(val, 0);
> > +               DBG("Csis Size: %u", val);
> > +               btd_opts.csis_defaults.cs_size = val;
> > +       }
> > +
> > +       val = g_key_file_get_integer(config, "CSIP", "CsisRank", &err);
> > +       if (err) {
> > +               DBG("%s", err->message);
> > +               g_clear_error(&err);
> > +       } else {
> > +               val = MIN(val, 0xFF);
> > +               val = MAX(val, 0);
> > +               DBG("Csis Rank: %u", val);
> > +               btd_opts.csis_defaults.cs_rank = val;
> > +       }
> > +
> >         str = g_key_file_get_string(config, "AVDTP", "SessionMode", &err);
> >         if (err) {
> >                 DBG("%s", err->message);
> > @@ -1014,6 +1121,12 @@ static void init_defaults(void)
> >         btd_opts.defaults.br.scan_type = 0xFFFF;
> >         btd_opts.defaults.le.enable_advmon_interleave_scan = 0xFF;
> >
> > +       btd_opts.csis_defaults.sirk_type = 1;
> > +       hex2bin(DEFAULT_SIRK, btd_opts.csis_defaults.sirk_val,
> > +                       sizeof(btd_opts.csis_defaults.sirk_val));
> > +       btd_opts.csis_defaults.cs_size = 1;
> > +       btd_opts.csis_defaults.cs_rank = 1;
> > +
> >         if (sscanf(VERSION, "%hhu.%hhu", &major, &minor) != 2)
> >                 return;
> >
> > diff --git a/src/main.conf b/src/main.conf
> > index f187c9aaa482..5378472ef0d4 100644
> > --- a/src/main.conf
> > +++ b/src/main.conf
> > @@ -258,6 +258,30 @@
> >  # Default to 3
> >  #Channels = 3
> >
> > +[CSIP]
>
> Let's use Set as a group name.
>
> > +# CSIP - Co-ordinated Set Identification Profile
> > +# SIRK Types which determines the value type for CsisSirkValue
> > +# Possible values:
> > +# 1 - Plain text
> > +# 2 - encrypted
> > +#CsisSirkType = 1
>
> Let's not use the type directly here so perhaps something like
> KeyType=plaintext,encrypted, etc, and it is important to say what is
> the default value.

Actually this doesn't make much sense, we can't really enter the key
in encrypt format since that has to be encrypted with device's LTK,
perhaps we should just have EncryptKey=y/n.

> > +
> > +# CSIP - Co-ordinated Set Identification Profile
> > +# SIRK - Set Identification resolution key which is common for all the
> > +# sets. They SIRK key is used to identify its sets. This can be any
> > +# 128 bit value.
> > +# Possible Values:
> > +# 16 byte hexadecimal value
> > +#CsisSirkValue = 861FAE703ED681F0C50B34155B6434FB
>
> Let's use Key instead, since this would belong to [Set] group that is
> enough to disambiguate.
>
> > +
> > +#CSIP - Size
> > +#Total no of sets belongs to this Profile
> > +#CsisSize = 1
>
> Size
>
> > +#CSIP - Rank
> > +#Rank for the device
> > +#CsisRank = 1
>
> Rank
>
> >  [AVDTP]
> >  # AVDTP L2CAP Signalling Channel Mode.
> >  # Possible values:
> > --
> > 2.25.1
> >
>
>
> --
> Luiz Augusto von Dentz


-- 
Luiz Augusto von Dentz