Hi Archie,
On Tue, Nov 2, 2021 at 2:10 AM Archie Pusaka <apusaka@xxxxxxxxxx> wrote:
>
> Hi Luiz,
>
>
> On Tue, 2 Nov 2021 at 14:17, Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
> >
> > Hi Archie,
> >
> > On Sun, Oct 31, 2021 at 11:06 PM Archie Pusaka <apusaka@xxxxxxxxxx> wrote:
> > >
> > > From: Archie Pusaka <apusaka@xxxxxxxxxxxx>
> > >
> > > Introducing PeripheralLongTermKey group for storing LTK info to
> > > replace the less inclusive term. Currently we still need to write/read
> > > from both to ensure smooth transition, but later we should deprecate
> > > the old term.
> > >
> > > Reviewed-by: Sonny Sasaka <sonnysasaka@xxxxxxxxxxxx>
> > > ---
> > >
> > > src/adapter.c | 41 ++++++++++++++++++++++++++++++++---------
> > > 1 file changed, 32 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/src/adapter.c b/src/adapter.c
> > > index d0d38621b8..6b12c9e793 100644
> > > --- a/src/adapter.c
> > > +++ b/src/adapter.c
> > > @@ -3868,7 +3868,11 @@ static struct smp_ltk_info *get_peripheral_ltk_info(GKeyFile *key_file,
> > >
> > > DBG("%s", peer);
> > >
> > > - ltk = get_ltk(key_file, peer, bdaddr_type, "SlaveLongTermKey");
> > > + /* Read from both entries. Later we should deprecate Slave. */
> > > + ltk = get_ltk(key_file, peer, bdaddr_type, "PeripheralLongTermKey");
> > > + if (!ltk)
> > > + ltk = get_ltk(key_file, peer, bdaddr_type, "SlaveLongTermKey");
> >
> > If you find the old name being used you better replace it with the new
> > one and erase the old.
> >
> > > if (ltk)
> > > ltk->central = false;
> > >
> > > @@ -8415,13 +8419,12 @@ static void new_link_key_callback(uint16_t index, uint16_t length,
> > > bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
> > > }
> > >
> > > -static void store_longtermkey(struct btd_adapter *adapter, const bdaddr_t *peer,
> > > +static void store_ltk_group(struct btd_adapter *adapter, const bdaddr_t *peer,
> > > uint8_t bdaddr_type, const unsigned char *key,
> > > - uint8_t central, uint8_t authenticated,
> > > + const char *group, uint8_t authenticated,
> > > uint8_t enc_size, uint16_t ediv,
> > > uint64_t rand)
> > > {
> > > - const char *group = central ? "LongTermKey" : "SlaveLongTermKey";
> > > char device_addr[18];
> > > char filename[PATH_MAX];
> > > GKeyFile *key_file;
> > > @@ -8431,11 +8434,6 @@ static void store_longtermkey(struct btd_adapter *adapter, const bdaddr_t *peer,
> > > char *str;
> > > int i;
> > >
> > > - if (central != 0x00 && central != 0x01) {
> > > - error("Unsupported LTK type %u", central);
> > > - return;
> > > - }
> > > -
> > > ba2str(peer, device_addr);
> > >
> > > snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
> > > @@ -8475,6 +8473,31 @@ static void store_longtermkey(struct btd_adapter *adapter, const bdaddr_t *peer,
> > > g_key_file_free(key_file);
> > > }
> > >
> > > +static void store_longtermkey(struct btd_adapter *adapter, const bdaddr_t *peer,
> > > + uint8_t bdaddr_type, const unsigned char *key,
> > > + uint8_t central, uint8_t authenticated,
> > > + uint8_t enc_size, uint16_t ediv,
> > > + uint64_t rand)
> > > +{
> > > + if (central != 0x00 && central != 0x01) {
> > > + error("Unsupported LTK type %u", central);
> > > + return;
> > > + }
> > > +
> > > + if (central) {
> > > + store_ltk_group(adapter, peer, bdaddr_type, key, "LongTermKey",
> > > + authenticated, enc_size, ediv, rand);
> > > + } else {
> > > + /* Store duplicates for now. Later we should deprecate Slave. */
> > > + store_ltk_group(adapter, peer, bdaddr_type, key,
> > > + "PeripheralLongTermKey", authenticated,
> > > + enc_size, ediv, rand);
> > > + store_ltk_group(adapter, peer, bdaddr_type, key,
> > > + "SlaveLongTermKey", authenticated,
> > > + enc_size, ediv, rand);
> >
> > In not following why you want to keep duplicate entries?
> >
> Based on the discussion in the other thread with Marcel, we don't want
> to immediately remove the old entry since this would cause friction
> when people are switching around between the newer and older version
> of BlueZ.
I guess the problem would be downgrading wouldn't work since we would
have removed the old value once upgrading, in that case I would be
fine keeping it but Id leave a comment since we should probably remove
that as soon as we have a few releases.
> Instead, for some time we would keep both entries alive, and only
> after that we remove the old entry.
>
> > > + }
> > > +}
> > > +
> > > static void new_long_term_key_callback(uint16_t index, uint16_t length,
> > > const void *param, void *user_data)
> > > {
> > > --
> > > 2.33.1.1089.g2158813163f-goog
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
> Thanks,
> Archie
--
Luiz Augusto von Dentz
