| Message ID | 20211101140544.Bluez.1.I515833d2764b8ec2ac2bb1f87313de80ebb497cd@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Bluez,1/3] adapter: Use PeripheralLongTermKey to store LTK | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/setupell | success | Setup ELL PASS |
| tedd_an/buildprep | success | Build Prep PASS |
| tedd_an/build | success | Build Configuration PASS |
| tedd_an/makecheck | success | Make Check PASS |
| tedd_an/makedistcheck | success | Make Distcheck PASS |
| tedd_an/build_extell | success | Build External ELL PASS |
| tedd_an/build_extell_make | success | Build Make with External ELL PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=573377 ---Test result--- Test Summary: CheckPatch PASS 4.10 seconds GitLint PASS 2.80 seconds Prep - Setup ELL PASS 42.57 seconds Build - Prep PASS 0.50 seconds Build - Configure PASS 7.91 seconds Build - Make PASS 182.22 seconds Make Check PASS 9.23 seconds Make Distcheck PASS 218.49 seconds Build w/ext ELL - Configure PASS 8.01 seconds Build w/ext ELL - Make PASS 173.06 seconds --- Regards, Linux Bluetooth
Hi Archie, On Sun, Oct 31, 2021 at 11:06 PM Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > 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@chromium.org> > --- > > 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? > + } > +} > + > 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 >
Hi Luiz, On Tue, 2 Nov 2021 at 14:17, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Archie, > > On Sun, Oct 31, 2021 at 11:06 PM Archie Pusaka <apusaka@google.com> wrote: > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > 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@chromium.org> > > --- > > > > 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. 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
Hi Archie, On Tue, Nov 2, 2021 at 2:10 AM Archie Pusaka <apusaka@google.com> wrote: > > Hi Luiz, > > > On Tue, 2 Nov 2021 at 14:17, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Archie, > > > > On Sun, Oct 31, 2021 at 11:06 PM Archie Pusaka <apusaka@google.com> wrote: > > > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > > > 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@chromium.org> > > > --- > > > > > > 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
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 (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); + } +} + static void new_long_term_key_callback(uint16_t index, uint16_t length, const void *param, void *user_data) {