Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth: Support for multiple local identity addresses #7473

Closed
4 tasks done
carlescufi opened this issue May 11, 2018 · 18 comments
Closed
4 tasks done

Bluetooth: Support for multiple local identity addresses #7473

carlescufi opened this issue May 11, 2018 · 18 comments
Assignees
Labels
area: Bluetooth Feature A planned feature with a milestone
Milestone

Comments

@carlescufi
Copy link
Member

carlescufi commented May 11, 2018

In order to support the HID usecase, the BLE Host needs to be able to switch its identity address on the fly.

The parts involved in order to make this possible are:

  • Storage scheme that is forward-compatible with multiple local ID addresses
  • APIs to switch local identity addresses or all APIs to include local ID address
  • Key and CCC handling per local ID address in GAP and GATT
  • Read/Write handling per local identity in GATT at the application level
@carlescufi carlescufi added the Feature A planned feature with a milestone label May 11, 2018
@carlescufi carlescufi added this to the v1.13 milestone May 11, 2018
@carlescufi
Copy link
Member Author

carlescufi commented May 11, 2018

@pizi-nordic and @jhn-nordic: Please detail the full requirements here so we can take a look
CC @oyvindronningstad

@jhedberg
Copy link
Member

jhedberg commented May 12, 2018

Assuming that we go with the approach of supporting multiple identity addresses, I'd propose extending the settings storage in the following forward-compatible way:

Currently the identity address is stored in the key bt/id and the local IRK in bt/irk. I.e. one bt_addr_le_t in the first one and one u8_t irk[16] in the second. What I'd do is to convert these to arrays, i.e. first one would be array of bt_addr_le_t and the second array of u8_t irk[16]. The index to the arrays would be the "identity identifier", which is something we'd also use in the code to refer to a specific identity. So identity 0 would be what we support right now.

The next step is to extend the bonding keys and CCCD storage. Currently these are stored in bt/keys/<remote addr> and bt/ccc/<remote addr>. I'd consider keeping these as the locations for identity 0. For other identities I'd just append the identity identifier, so e.g. bonding keys for identity 1 would be stored in bt/keys1/<remote addr>. In the code itself I'd use u8_t for the identifier, so there'd be a limit of max 256 identities, is that ok? This u8_t identifier would be present in struct bt_dev (e.g. to identify which identity we're advertising with), struct bt_conn (to identify which identity the connection was created with) as well as struct bt_keys. The existing bt_dev.id_addr and bt_dev.irk variables would become arrays, whose size would be determined by a new CONFIG_BT_IDENTITY_MAX Kconfig variable.

For the public API I'd propose something like:

int bt_identity_create(bt_addr_le_t *addr);

int bt_identity_get(u8_t id, bt_addr_le_t *addr);

These would fill in the bt_addr_le with the identity address, and return the identifier (index) of the identity (or a negative error number on error). The recently added bt_set_id_addr() could remain, and would keep operating on identity 0.

For removing an identity there could be something like:

int bt_identity_remove(u8_t id);

And, in the longer run, if we want to have something like was proposed by @holtmann on IRC, there could be a:

int bt_identity_set_privacy(u8_t id, bool enable);

To be able to select a specific identity to advertise with, I'd extend our struct bt_le_adv_opt with a u8_t identity member. This should keep any old code doing the "right thing" since any member not explicitly initialized defaults to 0 (well, depends on how the stack variable is declared and initialized, but it'd at least work the way our helper macros do it and how our existing sample code does it).

Thoughts?

@jhedberg
Copy link
Member

One thing I'm wondering, I didn't find it useful to expose the actual IRK value publicly, and perhaps we don't even need/want to expose the address itself either? I.e. all the app knows is that there's a certain set of identities or that it created a new one with a given id.

@jhedberg jhedberg changed the title BLE multiple local identity addresses Bluetooth: Support for multiple local identity addresses May 12, 2018
@carlescufi
Copy link
Member Author

Thanks @jhedberg for proposing this API. I think it makes sense. In general I think it is a very good proposal. Here are some comments:

  1. We still need to support setting the id address manually for each identity. This is not only because we already have an API, but also because hard-coded addresses in production are very common. In fact longer term we might want to think about the app pre-provisioning IRK and LTK as well, since as mentioned it is common to do this in "bundle" products. Exposing the IRK to the app is probably not required, but allowing it to set it is, later down the line.

  2. The way you suggest to store bonds is logical to me. For pre-provisioning we would need an additional bt_identity_set_local_keys(id, peer) or similar. In any case I don't see anything in the way that you are going to store the local addresses, IRKs or anything else that would be an issue here.

  3. The selection of identity needs to apply to scanning and connection creation down the line as well, so we will need to think about it.

@carlescufi
Copy link
Member Author

carlescufi commented May 14, 2018

@jhedberg why not:

bt_identity_create(bt_addr_le_t **addr, u8_t **irk);

If the addr/irk params are NULL, then the stack populates them. Otherwise the stack takes the values coming from the app.
EDIT: This still doesn't solve the public address one completely, since that comes from the controller itself. Maybe we need to separate the address type from the address pointer in this call.

@oyvindronningstad
Copy link
Collaborator

About the index:

  • Just to be sure: Editing of an identity should be forbidden altogether. Only creating (or generating) new ones should be allowed.
  • If you want to purge an identity, it might be risky to reuse the index for a new identity, even if you have deleted all the bonds associated with the purged identity since the user might have stored the index for its own purposes. Do we keep the entry in the array? In that case, the array might bloat badly in certain use cases. This could be solved e.g. by keeping an id_id (bad name :P) together with the identity.

@jhn-nordic
Copy link

Here are a description of the real life use cases that are the reason behind the need for an approach with multiple identities

Use case A:

kb_3hosts

Use case A1:

I have a BT enabled keyboard that supports only one bond at the time. I am now connected and bonded to Host A and want to pair with Host B. I press the pairing button on the keyboard. This operation deletes the bond the keyboard have to Host A (in the keyboard), and the keyboard starts advertising for a new bond. If we do not change our identity Host A will connect to keyboard immediately if within range and we cannot do a succesfull pairing with Host B.

Use case A2:

I have a BT enabled keyboard that can support up to i.e 3 bonds. The keyboard is only connected to one Host at the time, and it has 3 buttons that selects which host it should connect to. When searching for new bonds we will run into the same problems as in Usecase A1 If we do not operate with multiple identities

Use case A3:

I have a BT enabled keyboard that can support up to i.e 3 bonds The keyboard is CAN BE connected to all 3 hosts simultaneously, and it has 3 buttons or a host side SW that selects which host that shall receieve the HID inputs. When searching for new bonds we will run into the same problems as in Usecase A1 and A2 If we do not operate with multiple identities. However, in this use case we must also handle having simultaneous connections using different identites.

Use case B:

These are basically variants of the A3 use case with similar needs for multiple identies and connections.

remote_2hosts

Use case B1:

I have a BT enabled remote control that supports only one bond at the time. I am now connected and bonded to Host D and want to pair with Host E. I press the pairing button on the remote to start the pairing procedure towards Host E. However I still want the remote control to be fully operation towards Host D until the moment where the remote control is fully configured and bonded to Host E. Then HID input is routed to Host E and Host D is disconnected.

Use case B2:

I have a BT enabled multi remote control that supports multiple bonds The remote control is CAN BE connected to multiple hosts simultaneously (i.e. TV, set-top box, Hi-Fi, media centre), Different buttons can send HID events to one or multiple hosts. I.e a global "mute"-button could send the mute HID command to several of the hosts.

@carlescufi
Copy link
Member Author

since the user might have stored the index for its own purposes.

Not sure I follow. The user is the one deleting the identity, so it should know not to use that index anymore unless it creates a new one.

@oyvindronningstad
Copy link
Collaborator

since the user might have stored the index for its own purposes.

Not sure I follow. The user is the one deleting the identity, so it should know not to use that index anymore unless it creates a new one.

In practice, this is cumbersome because one part of the app doesn't know every other part. Being told that an identity is deprecated would make some things much easier.

@carlescufi
Copy link
Member Author

@jhn-nordic: Thanks for the use cases.

@carlescufi
Copy link
Member Author

I would like to cover the following possibilities here in this issue:

a) Single identity, multiple IRKs, per-peer
b) Multiple identities, multiple IRKs, per-identity

In each of the above the public address should be an identity, only if it exists.

Additionally I would like to cover pre-provisioning of bonding information. This can be done in 2 ways:

  1. Add APIs for it
  2. Add a sample that pre-populates the settings, from the application code, to add the required addresses, IRKs and LTKs.

I think I'd rather concentrate on 2) to avoid polluting the APIs.

@pizi-nordic
Copy link
Collaborator

pizi-nordic commented May 14, 2018

Hi @jhedberg, @carlescufi

Here you have some of my thoughts:

  • In my opinion application should have access to all IRKs for debug and testing purposes. If you would like to automate BT testing of device which could have multiple randomly generated identities without dedicated chamber, you have to be able to readout current device configuration.

  • IMHO we have to think a bit more about a higher-level problems. The separate identities might use different values in characteristics other than CCCDs. The HID Protocol Mode is an example. It would be great if stack could handle separation of such characteristics. What about advertising? Is simultaneous advertising using multiple identities possible? What about in directed advertising in such scenatrio?

@Vudentz
Copy link
Contributor

Vudentz commented May 14, 2018

@pizi-nordic bt_gatt_attr already takes the bt_conn as parameter with that the application should be able to tell what identity it is using. Also just to be clear here, when it comes to GATT database the stack only manages the attributes of the services it has registered, the only exception being CCCD when registered with BT_GATT_CCC and Id like to keep it like that, if you want any service to have advanced control using the identity addresses I guess we would need it to be part of the stack with kconfig entry to enable it, though it may be tricky with service that do require specialized application since that probably means profile specific public API on top of GATT.

@pizi-nordic
Copy link
Collaborator

Thanks @Vudentz. I just wanted to be sure that this feature will be easy to use from application standpoint.

@jhedberg
Copy link
Member

jhedberg commented Jul 4, 2018

Question: we have the public int bt_unpair(const bt_addr_le_t *addr); API. With the introduction of multiple identities, should this remain as-is, i.e. unpair addr for any identity it's found under. Or, should we break the API by extending it with an id parameter?

Another question: how important is it to extend this support for central role APIs? We have things like the following which may be affected:

struct bt_conn *bt_conn_create_le(const bt_addr_le_t *peer,
                                  const struct bt_le_conn_param *param);
int bt_le_set_auto_conn(bt_addr_le_t *addr,
                        const struct bt_le_conn_param *param);

Should we break those APIs as well? Another less intrusive alternative would be to have something like bt_le_central_set_id() to be called before either of the above two calls.

@jhn-nordic
Copy link

I think we need to know which identity we unpair from. Adding id as a parameter would work. As an example if you have a BLE keyboard that supports 3 bonds that you can select with 3 buttons you can do 3 pairings with the same host using 3 different identities (if you want). If you then call bt_unpair() with only the host address as a reference you would then unpair all the 3 bonds you have.

the bt_unpair() in the API is quite new and I guess the pain of changing the API now will be much less painful than doing it on later stage.

Regarding central role API I do not see a use for multiple IDs at the moment from my perspective (HID). Any comments @pizi-nordic @Qbicz ?

@Qbicz
Copy link
Collaborator

Qbicz commented Jul 5, 2018

I agree with @jhn-nordic. Regarding central role - it is probably not suffering from the limitation described in the HID use case, as usually the central initiates the connection. Moreover if central connects to one peripheral it does not prevent it from scanning and connecting to another peripheral. So for central having multiple identities is not that important.

@pizi-nordic
Copy link
Collaborator

At the moment I do not see any use case for multiple identities for central role.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Feature A planned feature with a milestone
Projects
None yet
Development

No branches or pull requests

8 participants